[PATCH] D51032: [ARM] Avoid injecting constant islands in movw+movt pairs on Windows

[Eli Friedman via Phabricator via llvm-commits]

efriedma added a comment.

> That's probably the best way forward, although I do feel that's probably a rather large change - I don't know off-hand where to start with that.

Probably just disable the code to expand it in ARMExpandPseudo if the target is Windows, add code to emit the pseudo-instruction in ARMAsmPrinter, and fix ARMBaseInstrInfo::getInstSizeInBytes to return the right result.  I don't think you'd actually need to change anything else; nothing should modify an unknown pseudo-instruction.

> Since nobody seems to have run into this before, I'm pondering if this fix could be acceptable until a proper restructuring with a pseudoinstruction can be done.

Looking a bit more closely, it looks like we bundle the instructions on Windows, so actually the window for other passes to mess with the unbundled pair is pretty small (just passes between unbundling and the asmprinter, which is basically just ConstantIslands).  So maybe this is good enough without a new pseudo-instruction.



================
Comment at: lib/Target/ARM/ARMConstantIslandPass.cpp:1427
+  // constant island is injected inbetween them, the relocation will clobber
+  // the instruction and fail to update the MOVT instruction.
+  if (STI->isTargetWindows() && isThumb && MI->getOpcode() == ARM::t2MOVTi16 &&
----------------
Probably should mention that the instructions are bundled until just before ConstantIslands.


================
Comment at: lib/Target/ARM/ARMConstantIslandPass.cpp:1429
+  if (STI->isTargetWindows() && isThumb && MI->getOpcode() == ARM::t2MOVTi16 &&
+      MI->getNumOperands() >= 3 &&
+      (MI->getOperand(2).getTargetFlags() & ARMII::MO_OPTION_MASK) ==
----------------
The "MI->getNumOperands() >= 3" is probably unnecessary.


================
Comment at: lib/Target/ARM/ARMConstantIslandPass.cpp:1432
+          ARMII::MO_HI16) {
+    --MI;
+  }
----------------
Assert that MI is a MOVW?


Repository:
  rL LLVM

https://reviews.llvm.org/D51032