[PATCH] D39173: [MachineCSE] Remove redudant TLS access instructions.

[Tony Jiang via Phabricator via llvm-commits]

jtony added inline comments.


================
Comment at: lib/CodeGen/MachineCSE.cpp:255
+    if (!MRI->isConstantPhysReg(Reg) &&
+        !(TRI->isCallerPreservedPhysReg(Reg, *MI->getParent()->getParent())))
       for (MCRegAliasIterator AI(Reg, TRI, true); AI.isValid(); ++AI)
----------------
hfinkel wrote:
> kbarton wrote:
> > Is it not possible to put this check into isConstantPhysReg instead? 
> > 
> No, I don't think so. We don't track register dependencies at all (e.g., during instruction scheduling) for constant phys regs. We can't add r2 to that constant set (because we need scheduling to understand the constraints on the r2 save/restore code).
> 
> isCallerPreservedPhysReg can only be used if we know that we're not dealing with transformations that can disturb the save/restore sequences. Is this generally safe here?
Thanks Hal for your quick reply.  I agree it is not safe to put the isCallerPreservedPhysReg check into isConstantPhysReg.  About your question, are you talking about any spill/reload code at all or you are just talking about prologue/epilogue? If you are talking about the prologue/epilogue, that (PEI) happens after the MCSE, so MCSE cannot disturb the save/restore sequences there (in PEI)  it should be safe to do so.


https://reviews.llvm.org/D39173