[PATCH] D67133: [GlobalISel]: Fix some non determinism exposed in CSE due to not notifying observers about mutations + add verification for CSE

[Aditya Nandakumar via Phabricator via llvm-commits]

aditya_nandakumar created this revision.
aditya_nandakumar added reviewers: arsenm, bogner, dsanders, aemerson, qcolombet, paquette, volkan.
Herald added subscribers: Petar.Avramovic, mgrang, hiraditya, kristof.beyls, tpr, javed.absar, rovka, wdng.
Herald added a project: LLVM.

While investigating some non determinism (CSE doesn't produce wrong code, it just doesn't CSE some times) in GISel CSE on an out of tree target, I realized that the core issue was that there were lots of code that mutates (setReg, setRegClass etc), but doesn't notify observers (CSE in this case but this could be any other observer). In order to make the Observer be available in various parts of code and to avoid having to thread it through various API, the MachineFunction now has the observer as field. This allows it to be easily used in helper functions such as constrainOperandRegClass. One of the other option I considered was to have some sort of ```unique_ptr<GISelContext> GISelCtxt``` and move the various GISel related properties into this as well but for now I didn't do this.

Also added some invariant verification method in CSEInfo which can catch these issues (when CSE is enabled). Right now it's at the end the legalizer and a few tests in AArch64,AMDGPU,X86 fail. I haven't fixed these yet, but will get to them hopefully this week while we get some eyes on this patch. Also it's not clear to me if this should be always run at the end of each pass (where CSE is enabled in DEBUG?)?

One of the options considered was to automatically track mutations but this is very invasive and the overhead is likely a lot.


Repository:
  rL LLVM

https://reviews.llvm.org/D67133

Files:
  llvm/include/llvm/CodeGen/GlobalISel/CSEInfo.h
  llvm/include/llvm/CodeGen/GlobalISel/GISelChangeObserver.h
  llvm/include/llvm/CodeGen/MachineFunction.h
  llvm/lib/CodeGen/GlobalISel/CSEInfo.cpp
  llvm/lib/CodeGen/GlobalISel/GISelChangeObserver.cpp
  llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp
  llvm/lib/CodeGen/GlobalISel/Legalizer.cpp
  llvm/lib/CodeGen/GlobalISel/Utils.cpp

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D67133.218553.patch
Type: text/x-patch
Size: 6922 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20190903/26ca2ee4/attachment.bin>