[llvm] r313925 - Enable the reuse of values computed in a previous loop iteration.

[Friedman, Eli via llvm-commits]

On 9/21/2017 3:45 PM, Friedman, Eli via llvm-commits wrote:
> Comments inline.
>
> On 9/21/2017 2:48 PM, Pranav Bhandarkar via llvm-commits wrote:
>> Author: pranavb
>> Date: Thu Sep 21 14:48:23 2017
>> New Revision: 313925
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=313925&view=rev
>> Log:
>> Enable the reuse of values computed in a previous loop iteration.
>>
>> This patch adds a pass that removes the computation of provably 
>> redundant
>> expressions that have been computed earlier in a previous iteration. It
>> relies on the use of PHIs to identify loop carried dependences.
>>
>> This is scalar replacement for vector types.
>
> I'm not sure it makes sense for this to be its own pass... it's very 
> similar to the scalar PRE done by GVN.
>
>> +bool 
>> HexagonVectorLoopCarriedReuse::isEquivalentOperation(Instruction *I1,
>> + Instruction *I2) {
>> +  if (!I1->isSameOperationAs(I2))
>> +    return false;
>> +  // This check is in place specifically for intrinsics. 
>> isSameOperationAs will
>> +  // return two for any two hexagon intrinsics because they are 
>> essentially the
>> +  // same instruciton (CallInst). We need to scratch the surface to 
>> see if they
>> +  // are calls to the same function.
>> +  if (CallInst *C1 = dyn_cast<CallInst>(I1)) {
>> +    if (CallInst *C2 = dyn_cast<CallInst>(I2)) {
>> +      if (C1->getCalledFunction() != C2->getCalledFunction())
>
> Given that you never create PHI nodes for constants, I think you need 
> to check that every constant operand matches, not just the callee of 
> calls.
>

Err, my description of this problem here isn't really right. Really, the 
issue is that the loop in 
HexagonVectorLoopCarriedReuse::findValueToReuse which calls 
isDepChainBtwn completely ignores operands which aren't Instructions.  
That means you'll treat two instructions as equivalent in cases where 
they actually aren't equivalent.  The getCalledFunction() works around 
this for one common case (the callee of a call), but doesn't cover the 
general case.

-Eli

-- 
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project