[PATCH] D112811: [ARM] implement LOAD_STACK_GUARD for remaining targets

[Nick Desaulniers via Phabricator via llvm-commits]

nickdesaulniers added inline comments.


================
Comment at: llvm/lib/Target/ARM/Thumb2InstrInfo.cpp:250-258
 void Thumb2InstrInfo::expandLoadStackGuard(
     MachineBasicBlock::iterator MI) const {
   MachineFunction &MF = *MI->getParent()->getParent();
-  if (MF.getTarget().isPositionIndependent())
+  const GlobalValue *GV =
+      cast<GlobalValue>((*MI->memoperands_begin())->getValue());
+
+  if (MF.getSubtarget<ARMSubtarget>().isGVInGOT(GV))
----------------
Does `Thumb1InstrInfo::expandLoadStackGuard` need a similar change?

If so, then all the callers of `ARMBaseInstrInfo::expandLoadStackGuardBase` seem to also compute the same value of `GV` that is rematerialized in `ARMBaseInstrInfo::expandLoadStackGuardBase`. At that point, I'd say let's change the function signature of `ARMBaseInstrInfo::expandLoadStackGuardBase` to accept the `GlobalValue*` we calculated in the caller.  Basically, let's DRY up `const GlobalValue *GV = cast<GlobalValue>((*MI->memoperands_begin())->getValue());` which seems repeated in callers and callee.

Though if we don't need this change for thumb[1] then perhaps we don't need to do that (or could simply pass `nullptr` for that interface change.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D112811/new/

https://reviews.llvm.org/D112811