[PATCH] D6629: x86: Emit LAHF/SAHF instead of PUSHF/POPF

JF Bastien via llvm-commits llvm-commits at lists.llvm.org
Sun Oct 4 11:46:10 PDT 2015


jfb added inline comments.

================
Comment at: llvm/trunk/lib/CodeGen/MachineInstrBundle.cpp:313
@@ -312,3 +312,3 @@
 
-    bool IsRegOrSuperReg = MOReg == Reg || TRI->isSubRegister(MOReg, Reg);
+    bool IsRegOrSuperReg = MOReg == Reg || TRI->isSuperRegister(MOReg, Reg);
     bool IsRegOrOverlapping = MOReg == Reg || TRI->regsOverlap(MOReg, Reg);
----------------
sanjoy wrote:
> Was this bit intentional?  I think (without understanding the surrounding code) this is contributing to PR25033.
Yes, this change was intentional: the code used to get confused about sub/super reg before and my patch would tickle the issues (because it uses AL and AH separately. I believe that the previous code had no clue about subregs, and so other code was simply incorrect but that never manifested because of this bug. I think my fix may tickle other bugs :-)

Michael Hordijk tracked down the problem in the mailing list:

The IR:
```
        CALL64pcrel32 <ga:@foo>, <regmask>, %RSP<imp-use>, %RSP<imp-def>,
%EAX<imp-def>
        %RAX<def> = MOV64rr %R14<kill>
```
So `CALL` defines `EAX`, and we're asking `computeRegisterLiveness` whether or
not `RAX` is live.  `computeRegisterLiveness` walks backwards and one thing it
looks for is whether the register is being defined:
```
    if (IsRegOrSuperReg) {
        PRI.Defines = true;     // Reg or a super-register is defined.
        if (!MO.isDead())
        AllDefsDead = false;
    }
```

And this is what determines if we (`RAX`) is a register or a super-register
of the register being defined (`lib/CodeGen/MachineInstrBundle.cpp`):
```
313:    bool IsRegOrSuperReg = MOReg == Reg || TRI->isSubRegister(MOReg,
Reg);
```
This will return false, as `RAX` is not a sub register of `EAX`.



Repository:
  rL LLVM

http://reviews.llvm.org/D6629





More information about the llvm-commits mailing list