[PATCH] D139679: [X86] Don't zero out %eax if both %al and %ah are used

[Bill Wendling via Phabricator via llvm-commits]

void added inline comments.


================
Comment at: llvm/lib/Target/X86/X86FrameLowering.cpp:499-501
 void X86FrameLowering::emitZeroCallUsedRegs(BitVector RegsToZero,
                                             MachineBasicBlock &MBB) const {
   const MachineFunction &MF = *MBB.getParent();
----------------
arsenm wrote:
> Why is this doing so much work? Why can't this use LivePhysRegUnits and addLiveOuts?
Is there an example of how to use them?


================
Comment at: llvm/lib/Target/X86/X86FrameLowering.cpp:527
 
+  // If an 8-bit register is live out, we don't want to clear either the high
+  // or low version of the register.
----------------
nickdesaulniers wrote:
> Is live out of the function itself? Or between basic blocks?
> 
> MachineRegisterInfo can answer whether a given register is live in `MachineRegisterInfo::isLiveIn`. I assume that's the same side of the coin for "is a given register live out of my predecessor?"  Perhaps that's better to query than looking into every operand of every instruction in the function?
> 
> MachineBasicBlock::liveouts() also gives you an iterator to a RegisterPairMask. Might be useful here?
MachineBasicBlock::liveouts() doesn't appear to work here.


================
Comment at: llvm/lib/Target/X86/X86FrameLowering.cpp:535
+      for (const auto &MO : MI.operands()) {
+        if (!MO.isReg())
+          continue;
----------------
arsenm wrote:
> Not checking for a def?
No. We're only checking if it's live on the return instruction. I won't be counted as a def.


================
Comment at: llvm/lib/Target/X86/X86FrameLowering.cpp:540-541
+        if (X86::GR8RegClass.contains(Reg)) {
+          RegsToZero.reset(getX86SubSuperRegister(Reg, 8, false/*low*/));
+          RegsToZero.reset(getX86SubSuperRegister(Reg, 8, true /*high*/));
+        }
----------------
nickdesaulniers wrote:
> Is it possible to never set the bits of `RegsToZero` in `PEI::insertZeroCallUsedRegs()` for these in the first place?
There doesn't seem to be an easy mechanism for doing that. The iterator doesn't appear to go over both the high and low registers. I'm not sure if that's a bug or by design.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139679