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

[Nick Desaulniers via Phabricator via llvm-commits]

nickdesaulniers added a subscriber: RKSimon.
nickdesaulniers added a comment.

> The iterator over super and sub registers doesn't include both 8-bit registers in its list.

This is the use of `sub_and_superregs_inclusive` in the caller `PEI::insertZeroCallUsedRegs()` that you're referring to? Does that need to be fixed? Is there another iterator that gives us both? Should there be?  Maybe @pengfei , @RKSimon , or @craig.topper knows?

The generated `TargetRegisterClass` for `GR8RegClass` and `GRH8RegClass` BOTH have set `CoveredBySubRegs` to `false`.



================
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.
----------------
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?


================
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*/));
+        }
----------------
Is it possible to never set the bits of `RegsToZero` in `PEI::insertZeroCallUsedRegs()` for these in the first place?


================
Comment at: llvm/test/CodeGen/X86/zero-call-used-regs-i386.ll:20
+; I386-NEXT:    pushl %ebx
+; I386-NEXT:    .cfi_def_cfa_offset 8
+; I386-NEXT:    subl $24, %esp
----------------
add `nounwind` to to the attribute list of `@test1` to remove these obnoxious .cfi directives which are just noise.

Though, I do also wonder if a MIR test (`-stop-after=prologepilog`) would be less brittle/dependent on `%al` being allocated.  Then we could have 1 function per register and know precisely what happens for which registers.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139679