[llvm-commits] SHR Patch (Updated) #3/3 instcombine part

[Chris Lattner]

On Nov 2, 2006, at 3:33 PM, Reid Spencer wrote:

> Here are the SHR patches updated for recent changes on the HEAD. These
> replace the previous submission on Tuesday.  If you're reviewing  
> the SHR
> patches, please use this version now.
>
> ***NOTE: This is for review only, please don't commit any of this.

@@ -1143,52 +1175,40 @@ bool InstCombiner::SimplifyDemandedBits(
...
+      // If the input sign bit is known to be zero, or if none of  
the top bits
+      // are demanded, turn this into an unsigned shift right.
+      if ((KnownZero & SignBit) || (HighBits & ~DemandedMask) ==  
HighBits) {
+        // Convert the input to unsigned.
+        Value *NewVal = InsertCastBefore(I->getOperand(0),
+                           I->getType()->getUnsignedVersion(), *I);
+        // Perform the unsigned shift right.
+        NewVal = new ShiftInst(Instruction::LShr, NewVal, SA, I- 
 >getName());
+        InsertNewInstBefore(cast<Instruction>(NewVal), *I);
+        // Then cast that to the destination type.
+        NewVal = new CastInst(NewVal, I->getType(), I->getName());
+        InsertNewInstBefore(cast<Instruction>(NewVal), *I);
+        return UpdateValueUsesWith(I, NewVal);


No need for the casts: just replace the single instruction with an lshr.


@@ -1897,33 +1917,50 @@ Instruction *InstCombiner::visitSub(Bina
      // -((uint)X >> 31) -> ((int)X >> 31)
      // -((int)X >> 31) -> ((uint)X >> 31)
      if (C->isNullValue()) {
        Value *NoopCastedRHS = RemoveNoopCast(Op1);
        if (ShiftInst *SI = dyn_cast<ShiftInst>(NoopCastedRHS))
-        if (SI->getOpcode() == Instruction::Shr)
+        if (SI->getOpcode() == Instruction::LShr) {
            if (ConstantInt *CU = dyn_cast<ConstantInt>(SI->getOperand 
(1))) {
-            const Type *NewTy;
-            if (SI->getType()->isSigned())
-              NewTy = SI->getType()->getUnsignedVersion();
-            else
-              NewTy = SI->getType()->getSignedVersion();
+            const Type *NewTy = SI->getType()->getSignedVersion();
              // Check to see if we are shifting out everything but  
the sign bit.
              if (CU->getZExtValue() ==
                  SI->getType()->getPrimitiveSizeInBits()-1) {
                // Ok, the transformation is safe.  Insert a cast of  
the incoming
                // value, then the new shift, then the new cast.
                Value *InV = InsertCastBefore(SI->getOperand(0),  
NewTy, I);
-              Instruction *NewShift = new ShiftInst 
(Instruction::Shr, InV,
+              Instruction *NewShift = new ShiftInst 
(Instruction::AShr, InV,
                                                      CU, SI->getName 
());
                if (NewShift->getType() == I.getType())
                  return NewShift;
                else {
                  InsertNewInstBefore(NewShift, I);
                  return new CastInst(NewShift, I.getType());
                }
              }
            }
+        }
+        else if (SI->getOpcode() == Instruction::AShr) {
+          if (ConstantInt *CU = dyn_cast<ConstantInt>(SI->getOperand 
(1))) {
+            const Type *NewTy = SI->getType()->getUnsignedVersion();
+            // Check to see if we are shifting out everything but  
the sign bit.
+            if (CU->getZExtValue() ==
+                SI->getType()->getPrimitiveSizeInBits()-1) {
+              // Ok, the transformation is safe.  Insert a cast of  
the incoming
+              // value, then the new shift, then the new cast.
+              Value *InV = InsertCastBefore(SI->getOperand(0),  
NewTy, I);
+              Instruction *NewShift = new ShiftInst 
(Instruction::LShr, InV,
+                                                    CU, SI->getName());
+              if (NewShift->getType() == I.getType())
+                return NewShift;
+              else {
+                InsertNewInstBefore(NewShift, I);
+                return new CastInst(NewShift, I.getType());
+              }
+            }
+          }
+        }

*** Likewise here, your transformation is fine, but please eliminate  
the cast insertion.


@@ -2292,11 +2329,11 @@ Instruction *InstCombiner::visitUDiv(Bin
              N = InsertCastBefore(N, NTy, I);
            }
            Constant *C2V = ConstantInt::get(NTy, C2);
            N = InsertNewInstBefore(BinaryOperator::createAdd(N, C2V,  
"tmp"), I);
          }
-        Instruction* Result = new ShiftInst(Instruction::Shr, Op0, N);
+        Instruction* Result = new ShiftInst(Instruction::LShr, Op0, N);
          if (!isSigned)
            return Result;
          InsertNewInstBefore(Result, I);
          return new CastInst(Result, NTy->getSignedVersion(),  
I.getName());
        }

This hunk should be more aggressive, to eliminate cast insertion.   
I'd like to see something like this (this is a quick hack, please  
review it for correctness):

       // If the setcc is true iff the sign bit of X is set, then  
convert this
       // multiply into a shift/and combination.
       if (isa<ConstantInt>(SCIOp1) &&
           isSignBitCheck(SCI->getOpcode(), SCIOp0, cast<ConstantInt> 
(SCIOp1))) {
         // Shift the X value right to turn it into "all signbits".
         Constant *Amt = ConstantInt::get(Type::UByteTy,
                                           SCOpTy- 
 >getPrimitiveSizeInBits()-1);
         Value *V =
           InsertNewInstBefore(new ShiftInst(Instruction::LShr,  
SCIOp0, Amt,
                                             BoolCast->getOperand(0)- 
 >getName()+
                                             ".mask"), I);

         // If the multiply type is not the same as the source type,  
sign extend
         // or truncate to the multiply type.
         if (I.getType() != V->getType()) {
           if (V->getType()->isUnsigned())
             V = InsertCastBefore(V, V->getType()->getSignedVersion 
(), I);

           V = InsertCastBefore(V, I.getType(), I);
         }


@@ -2320,17 +2357,17 @@ Instruction *InstCombiner::visitUDiv(Bin
              if (isSigned)
                X = InsertCastBefore(X, X->getType()- 
 >getUnsignedVersion(), I);
              // Construct the "on true" case of the select
              Constant *TC = ConstantInt::get(Type::UByteTy, TSA);
              Instruction *TSI =
-              new ShiftInst(Instruction::Shr, X, TC, SI->getName() 
+".t");
+              new ShiftInst(Instruction::LShr, X, TC, SI->getName() 
+".t");
              TSI = InsertNewInstBefore(TSI, I);

Likewise.




It looks like there are several other places (that I didn't list)  
where you could more aggressively eliminate casts.  Please tackle  
these, then resend the patch for review.

Thanks,

-Chris


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