ardb added a comment. I have split off the LOAD_STACK_GUARD changes into
[ARM] implement LOAD_STACK_GUARD for remaining targets ================ Comment at: llvm/lib/Target/ARM/ARMBaseInstrInfo.cpp:4900 + .addImm(15) + .addImm(0) + .addImm(13) ---------------- nickdesaulniers wrote: > I think we want to use `Reg` in this instruction, in order to load into the > specified destination? We use Reg, no? MRC does not take reg operands, but I pass it as the target register. ================ Comment at: llvm/lib/Target/ARM/ARMBaseInstrInfo.cpp:4904 + .addImm(3) + .add(predOps(ARMCC::AL)); ---------------- nickdesaulniers wrote: > do we need to add `al`? It appears so. ================ Comment at: llvm/lib/Target/ARM/ARMInstrInfo.cpp:102 + if (M.getStackProtectorGuard() == "tls") { + expandLoadStackGuardBase(MI, ARM::MRC, ARM::LDRi12); + return; ---------------- nickdesaulniers wrote: > do we have to worry about soft tp, ie. `__aeabi_read_tp` vs `mrc`? I think we should only allow this hard TP is supported in the first place. I'll add a check somewhere. Calling a helper in both the prologue and the epilogue for emulated TLS seems a bit too heavyweight to me, and the Linux kernel will not use it in that way anyway. ================ Comment at: llvm/lib/Target/ARM/Thumb2InstrInfo.cpp:250 void Thumb2InstrInfo::expandLoadStackGuard( MachineBasicBlock::iterator MI) const { ---------------- nickdesaulniers wrote: > what about `Thumb1InstrInfo::expandLoadStackGuard`? Do we have `mrc` > available in Thumb[1] as well? No Thumb1 does not have an encoding for MRC so we can ignore it here. It does mean we should not allow this feature to be turned out for such targets. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112768/new/ https://reviews.llvm.org/D112768 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits