[PATCH] D31287: [mips] Fix atomic operations at O0, v3

[James Y Knight via Phabricator via llvm-commits]

jyknight added inline comments.


================
Comment at: lib/Target/Mips/MipsTargetMachine.cpp:288-290
+void MipsPassConfig::addPreSched2() {
+  addPass(createMipsExpandPseudoPass());
+}
----------------
sdardis wrote:
> sdardis wrote:
> > asb wrote:
> > > Would it be more robust to have this pass under addPreEmit2? This guarantees it runs after potentially troublesome passes such as the machineoutliner (obviously not yet enabled for Mips anyway), and you'd no longer need to worry about machine block placement.
> > I hadn't considered that yet. Most of my concerns were about making sure the register allocator didn't break the scratch registers by making them non-unique with the address or data registers for the pseudo instruction.
> After looking at the machine outliner, I can see that it's possible to use the getMachineOutlinerMBBFlags function to detect ll/sc sequences and mark those entire blocks as illegal to outline*.
> 
> The choice of preEmit2 is more complex for MIPS as we would then need to perform the equivalent of MipsBranchExpansion at the MC layer as we would have invalidated its' assumptions, along with the delay slot filler and the size reduction pass.
> 
> PreEmit seems to be the right place.
> 
> * ISTM that if the registers used by all the occurrences of a single type of atomic operation were the same, then they could be merged into a single outlined function.
The problem being that MipsBranchExpansion expects that every MI (by the time it runs) turns into 4 bytes of output? I think that just means these pseudos must be handled in getInstSizeInBytes(), right?

I think ideally the ll/sc loop should be treated as if it was effectively an inline-asm blob -- no mutations made by any other passes to the contents. So, e.g. if you want a delay slot filled, you place an instruction there yourself.

And, yes, it's perfectly reasonable to outline the LL/SC loop in its entirety, just not _parts_ of it.

But maybe this discussion should be continued on the llvm-dev email thread. Certainly moving to preEmit is closer to the ideal state than before. :)


Repository:
  rL LLVM

https://reviews.llvm.org/D31287