[llvm] r293058 - [InstCombine] Canonicalize guards for AND condition

[Artur Pilipenko via llvm-commits]

> Jan 27, 2017, в 6:30 AM, Sanjoy Das <sanjoy at playingwithpointers.com> написал(а):
> 
> Hi,
> 
> I had to revert these changes in https://reviews.llvm.org/rL293227
> since they're buggy around guard intrinsics carrying extra arguments.
> I have an example in the commit message for the revert commit.
Maxim, the author of the patches, is currently working on a fix for that. 
> 
> Thinking about this a bit, I'm also not convinced of the utility
> of (2) and (3) in PR31520.  InstCombine does the same canonicalization
> for assumes, but there is a fundamental difference between guards and
> assumes: assumes do not imply a runtime check of any form, and
> complicated expressions feeding into an assumes makes it difficult to
> compute the set of ephemeral values.  However, values feeding into a
> guard are not ephemeral and the same reasoning does not apply.  In
> fact, the extra uses of deopt state inserted by duplicating guards may
> actually be a pessimization (since certain InstCombine optimizations
> kick in if there is only one use of a value).
I think it still can be a useful canonicalization since it enables simplification of all the code which analyze/transform guards. I believe most of this code now handles and-joined conditions. On the other hand I agree that in the resulting code it is better to have a single deopt rather than a bunch of them. If we decide to canonicalize this way we might need to do some reverse transformation right before lowering. 

Artur
> 
> -- Sanjoy
> 
> On Wed, Jan 25, 2017 at 6:20 AM, Artur Pilipenko via llvm-commits
> <llvm-commits at lists.llvm.org> wrote:
>> Author: apilipenko
>> Date: Wed Jan 25 08:20:52 2017
>> New Revision: 293058
>> 
>> URL: http://llvm.org/viewvc/llvm-project?rev=293058&view=rev
>> Log:
>> [InstCombine] Canonicalize guards for AND condition
>> 
>> This is a partial fix for Bug 31520 - [guards] canonicalize guards in instcombine
>> 
>> Reviewed By: apilipenko
>> 
>> Differential Revision: https://reviews.llvm.org/D29074
>> 
>> Patch by Maxim Kazantsev.
>> 
>> Modified:
>>    llvm/trunk/lib/Transforms/InstCombine/InstCombineCalls.cpp
>>    llvm/trunk/test/Transforms/InstCombine/call-guard.ll
>> 
>> Modified: llvm/trunk/lib/Transforms/InstCombine/InstCombineCalls.cpp
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/InstCombine/InstCombineCalls.cpp?rev=293058&r1=293057&r2=293058&view=diff
>> ==============================================================================
>> --- llvm/trunk/lib/Transforms/InstCombine/InstCombineCalls.cpp (original)
>> +++ llvm/trunk/lib/Transforms/InstCombine/InstCombineCalls.cpp Wed Jan 25 08:20:52 2017
>> @@ -2878,6 +2878,23 @@ Instruction *InstCombiner::visitCallInst
>>     if (match(II->getNextNode(),
>>               m_Intrinsic<Intrinsic::experimental_guard>(m_Specific(IIOperand))))
>>       return eraseInstFromFunction(*II);
>> +
>> +    // Canonicalize guard(a && b) -> guard(a); guard(b);
>> +    // Note: New guard intrinsics created here are registered by
>> +    // the InstCombineIRInserter object.
>> +    Function *GuardIntrinsic = II->getCalledFunction();
>> +    Value *A, *B;
>> +    OperandBundleDef DeoptOB(*II->getOperandBundle(LLVMContext::OB_deopt));
>> +    if (match(IIOperand, m_And(m_Value(A), m_Value(B)))) {
>> +      CallInst *GuardA =
>> +          Builder->CreateCall(GuardIntrinsic, A, {DeoptOB}, II->getName());
>> +      CallInst *GuardB =
>> +          Builder->CreateCall(GuardIntrinsic, B, {DeoptOB}, II->getName());
>> +      auto CC = II->getCallingConv();
>> +      GuardA->setCallingConv(CC);
>> +      GuardB->setCallingConv(CC);
>> +      return eraseInstFromFunction(*II);
>> +    }
>>     break;
>>   }
>>   }
>> 
>> Modified: llvm/trunk/test/Transforms/InstCombine/call-guard.ll
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/InstCombine/call-guard.ll?rev=293058&r1=293057&r2=293058&view=diff
>> ==============================================================================
>> --- llvm/trunk/test/Transforms/InstCombine/call-guard.ll (original)
>> +++ llvm/trunk/test/Transforms/InstCombine/call-guard.ll Wed Jan 25 08:20:52 2017
>> @@ -28,3 +28,23 @@ define void @test_guard_adjacent_neg(i1
>>   call void(i1, ...) @llvm.experimental.guard( i1 %B )[ "deopt"() ]
>>   ret void
>> }
>> +
>> +define void @test_guard_and(i1 %A, i1 %B) {
>> +; CHECK-LABEL: @test_guard_and(
>> +; CHECK-NEXT:    call void (i1, ...) @llvm.experimental.guard(i1 %A) [ "deopt"() ]
>> +; CHECK-NEXT:    call void (i1, ...) @llvm.experimental.guard(i1 %B) [ "deopt"() ]
>> +; CHECK-NEXT:    ret void
>> +  %C = and i1 %A, %B
>> +  call void(i1, ...) @llvm.experimental.guard( i1 %C )[ "deopt"() ]
>> +  ret void
>> +}
>> +
>> +define void @test_guard_and_non_default_cc(i1 %A, i1 %B) {
>> +; CHECK-LABEL: @test_guard_and_non_default_cc(
>> +; CHECK-NEXT:    call cc99 void (i1, ...) @llvm.experimental.guard(i1 %A) [ "deopt"() ]
>> +; CHECK-NEXT:    call cc99 void (i1, ...) @llvm.experimental.guard(i1 %B) [ "deopt"() ]
>> +; CHECK-NEXT:    ret void
>> +  %C = and i1 %A, %B
>> +  call cc99 void(i1, ...) @llvm.experimental.guard( i1 %C )[ "deopt"() ]
>> +  ret void
>> +}
>> 
>> 
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits