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

[Martin Storsjö via Phabricator via llvm-commits]

mstorsjo added a comment.

In https://reviews.llvm.org/D51032#1208359, @efriedma wrote:

> > 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.


Ah, thanks - that explains why it hasn't exploded completely before. I've only run into this with compiling LLVM itself for this target, where certain source files seemed to trigger it.



================
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 &&
----------------
efriedma wrote:
> Probably should mention that the instructions are bundled until just before ConstantIslands.
Ok, will do


================
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) ==
----------------
efriedma wrote:
> The "MI->getNumOperands() >= 3" is probably unnecessary.
Will remove


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


Repository:
  rL LLVM

https://reviews.llvm.org/D51032