Accidental null pointer to instruction in FNeg handling

[andrew-wja]

Hi folks,

LLVM 12 has added UnaryOperations and a single inhabitant of that class, the FNeg instruction. I've added support for this in 7217274.

When I uncomment the relevant lines in the tests 7217274#diff-31c16b814c466b0c4701c16c3ce1f576f9d84ed9edebdb10247b173666bdcd24R80-R86 I hit an assert in LLVM C++ where a null pointer is being passed as an Instruction* -- but I can't figure out why that is happening! It is triggered by the lowering of the Haskell FNeg data constructor through the FFI, which is handled in the TH code, and calls LLVM_hs_build_FNeg, which seems to work as it should.

Can you see anything I've missed in the commit? I had to add to the TH code a little (UnaryOperations are a new InstructionKind) but I think I've done that part correctly. Any help would be much appreciated!

[andrew-wja]

Just an update since I've put a few hours into this recently: I have no idea what's going on. I've stepped through the whole process of creating the instruction, and at every point where I've put in checks (or asserts in the FFI code) things appear to be OK.

As an aside, from doing this investigation I've come to believe that the hsc and Template Haskell code is a bit too clever for its own good. It makes it really difficult to debug this kind of issue, because there are at least three domain specific languages involved in the generation of the ultimate code that needs inspection.

It appears that the reason we have the hsc and TH code is so that we can generate a chunk of the FFI code from LLVM's def files. However, there are only a small handful of templated instructions, and the rest are still written out explicitly by hand! The burden of maintaining and debugging the multilevel templating is very high and the utility feels very low given the complexity cost we're paying for it.

It appears after a few hours of walking the codebase looking for the cause of this issue that it would be beneficial to just get rid of the hsc and TH dependencies entirely by refactoring the FFI layer slightly. It also appears that the FFI layer uses a weird mishmash of both the LLVM C and C++ APIs. After looking at the doxygen for LLVM 12 it seems that this is totally unnecessary, and in fact, the C API now takes care of several concerns that we've manually handled around maintaining builder state. I realise that the FFI code has been around for a very long time, so maybe it's time to consider a full refactor with the aim of simplifying everything so that people can more easily work on the FFI layer. I'll make an issue with a sketch of the changes to track this.