[llvm-commits] [llvm] r113670 - in /llvm/trunk: include/llvm/Target/TargetInstrInfo.h lib/CodeGen/PeepholeOptimizer.cpp lib/Target/ARM/ARMBaseInstrInfo.cpp lib/Target/ARM/ARMBaseInstrInfo.h

[Bill Wendling]

On Sep 13, 2010, at 11:04 AM, Gabor Greif wrote:

> On 12 Sep., 02:04, Bill Wendling <wendl... at apple.com> wrote:
>> I think the TST instruction can take a compare value other than 0. The CMP instructions take 0, right?
> TST instructions have 2 operands and the second is a mask. It is a
> hybrid of "and" and "cmp" against 0.

The third operand is a "shift" when using a register-register test (according to my documentation). But we covered that in a previous email.

>> Below is what I have in my tree right now. It's just the skeleton of how I'd like to handle this (it does nothing right now). I like it because of how it separates out different optimization potentialities for each type of instruction, but it may be overkill...
> 
> Yes, I think it is a bit of overkill. Logically TST is a compare
> instruction. In my last patch I successfully handle both with one
> sweep. If you think my approach is sufficient and works well on your
> testcases, I gladly check it in. Then I would like to establish
> something like
> 
> dyn_cast<ARMri<ARM::ANDri> >
> queries (with additional intelligence) for checking opcodes and
> getting operands. Would simplify my code a great amount.
> 
Here's your patch from another email. My comments are in-line:

gabor at google8:~/llvm-build$ svn diff /home/gabor/llvm/lib/Target/ARM/
ARMBaseInstrInfo.cpp
Index: /home/gabor/llvm/lib/Target/ARM/ARMBaseInstrInfo.cpp
===================================================================
--- /home/gabor/llvm/lib/Target/ARM/ARMBaseInstrInfo.cpp
(revision 113747)
+++ /home/gabor/llvm/lib/Target/ARM/ARMBaseInstrInfo.cpp
(working copy)
@@ -1372,6 +1372,19 @@
    SrcReg = MI->getOperand(0).getReg();
    CmpValue = MI->getOperand(1).getImm();
    return true;
+  case ARM::TSTri: {
+      if (MI->getParent()->begin() == MachineBasicBlock::const_iterator(MI))
+        return false;
+      const MachineInstr *AND = llvm::prior(MI);

The AND doesn't have to be the immediately prior instruction. I could happen further up in the BB just as long as there isn't an intervening instruction which sets CPSR.

+      if (AND->getOpcode() != ARM::ANDri)
+        return false;
+      if (MI->getOperand(0).getReg() == AND->getOperand(1).getReg() && // FIXME: or == AND
+          MI->getOperand(1).getImm() == AND->getOperand(2).getImm()) {// FIXME: subset
+        SrcReg = AND->getOperand(0).getReg();
+        CmpValue = 0;

CmpValue might not be 0 here.

+        return true;
+      }
+    }

There's no need for this to be as complex here. The logic should be pretty much identical to the CMP cases. In fact, I think it is identical when you have a TST of an immediate. This logic should reside in ARMBaseInstrInfo::OptimizeCompareInstr. Once you do that, the tests of SrcReg & CmpValue become trivial.

I need to comment this method better. It's supposed to return the source register and compare values of the comparison instruction.

  }

  return false;
@@ -1421,6 +1434,7 @@
  switch (MI->getOpcode()) {
  default: break;
  case ARM::ADDri:
+  case ARM::ANDri:

Also do t2ANDri.

  case ARM::SUBri:
  case ARM::t2ADDri:
  case ARM::t2SUBri:

To do the optimization you suggest should be almost as easy as simply adding the TST* cases to the switch statements and removing the "if (CmpValue != 0) return false;" from OptimizeCompareInstr. :-)

-bw

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20100913/af28af54/attachment.html>