[PATCH] D47655: [MachineOutliner] Don't outline sequences where x16/x17/nzcv are live across

[Eli Friedman via Phabricator via llvm-commits]

efriedma added inline comments.


================
Comment at: include/llvm/CodeGen/MachineOutliner.h:170
+    std::for_each((MachineBasicBlock::reverse_iterator)front(), MBB->rend(),
+                  [this](MachineInstr &MI) { LRUIn.stepBackward(MI); });
+  }
----------------
paquette wrote:
> efriedma wrote:
> > This could get expensive with a long basic block; I guess we can try to add some sort of cache later if it becomes an issue.
> If it'd be possible to infer candidate liveness from the entire block's liveness, we could wrap the candidate's MBB in a struct like
> 
> ```
> struct CandidateMBB
> {
>     MachineBasicBlock *MBB;
>     LiveRegUnits LRU;
>     ...
> };
> ```
> 
> Then we wouldn't risk recalculating liveness on one basic block over and over again in the case of, say, overlapping candidates. 
I don't think you can do anything useful at a per-block level, unless you want to try to compute some sort of approximation for long blocks.  (I wouldn't suggest the approximation approach, though; probably just as easy to keep around one LiveRegUnits for every 100 instructions or so.)


================
Comment at: lib/Target/AArch64/AArch64InstrInfo.cpp:4967
+  // x16 was defined inside the outlining candidate region, we cannot say that
+  // the value of x16 at Ik will be the same there.
+  //
----------------
Outdated comment.


================
Comment at: lib/Target/AArch64/AArch64InstrInfo.cpp:5011
+  // instructions that use LR that could possibly appear in a repeated sequence
+  // are calls and ADRPs. Therefore, we only have to check and see if LR is
+  // dead on entry to (or exit from) some candidate.
----------------
ADRP doesn't use LR, as far as I know.


https://reviews.llvm.org/D47655