yroux added a comment.
Hi Sam,
Thanks for your comments.
================
Comment at: llvm/lib/Target/ARM/ARMBaseInstrInfo.cpp:5606
+ // candidates.
+ auto CantGuaranteeValueAcrossCall = [&TRI](outliner::Candidate &C) {
+ // If the unsafe registers in this block are all dead, then we don't need
----------------
samparker wrote:
> Does this code work with the DSP instructions that read/write the Q and GE
> flags? I have a nasty feeling that we don't model their register usage.
Good question, I'll test that
================
Comment at: llvm/lib/Target/ARM/ARMBaseInstrInfo.cpp:5765
+ if (Opc == ARM::BX_RET || Opc == ARM::tBX_RET || Opc == ARM::MOVPCLR) {
+ if (MI.getOperand(0).getImm() != ARMCC::AL)
+ return outliner::InstrType::Illegal;
----------------
samparker wrote:
> There's the getPredicate helper is ARMBaseInstrInfo which looks like it would
> be useful here.
Ah yes, and there is even isPredicated which might simplify the logic here.
================
Comment at: llvm/lib/Target/ARM/ARMBaseInstrInfo.cpp:5801
+
+ // Don't touch the link register
+ if (MI.readsRegister(ARM::LR, TRI) || MI.modifiesRegister(ARM::LR, TRI))
----------------
samparker wrote:
> Probably worth doing these simple checks earlier. Should we also be concerned
> about the PC too?
Well, the phasing is important here, if we move the checking of LR or SP usage
earlier it will make the outlining of calls illegal since they implicitly
modifies these registers or we can go back to my previous version which was
checking the explicit usage but it introduces redundancy.
hmm.. maybe we can be conservative with PC, it is not verified in AArch64 code
@paquette do you think it can be an issue?
================
Comment at: llvm/lib/Target/ARM/ARMTargetMachine.cpp:553
addPass(createARMConstantIslandPass());
- addPass(createARMLowOverheadLoopsPass());
+ if (!MachineOutlinerEnabled)
+ addPass(createARMLowOverheadLoopsPass());
----------------
samparker wrote:
> We'll need the LowOverheadLoops pass to run for correctness, so we should
> instead only add the MachineOutliner if the subtarget doesn't support LOB.
What do you mean by "for correctness" ?
I think that it makes more sense that until MachineOutliner and
LowOverheadLoops can work together, we have loloops enabled on targets which
have LOB support unless it is explicitly disabled by -disable-arm-loloops flag
or if the user wants machine outlining with the flag -moutline. If we do that
in the opposite way it means that passing the flag -moutline will have no
impact on such targets unless the -disable-arm-loloops flag is used
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D76066/new/
https://reviews.llvm.org/D76066
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
