r201439 - [CodeGenPrepare][AddressingModeMatcher] Give up on type promotion if the

[Quentin Colombet]

Hi Tom,

On Feb 18, 2014, at 9:21 AM, Tom Stellard <tom at stellard.net> wrote:

> Hi Quentin,
> 
> Sorry I missed this on the first time around: 
> 
>> --- a/lib/Transforms/Scalar/CodeGenPrepare.cpp
>> +++ b/lib/Transforms/Scalar/CodeGenPrepare.cpp
>> @@ -1728,6 +1730,35 @@
>> TypePromotionHelper::promoteOperandForOther(Instruction *SExt,
>>  return SExtOpnd;
>> }
>> 
>> +/// IsPromotionProfitable - Check whether or not promoting an
>> instruction
>> +/// to a wider type was profitable.
>> +/// \p MatchedSize gives the number of instructions that have been
>> matched
>> +/// in the addressing mode after the promotion was applied.
>> +/// \p SizeWithPromotion gives the number of created instructions for
>> +/// the promotion plus the number of instructions that have been
>> +/// matched in the addressing mode before the promotion.
>> +/// \p PromotedOperand is the value that has been promoted.
>> +/// \return True if the promotion is profitable, false otherwise.
>> +bool
>> +AddressingModeMatcher::IsPromotionProfitable(unsigned MatchedSize,
>> +                                             unsigned SizeWithPromotion,
>> +                                             Value *PromotedOperand) const {
>> +  // We folded less instructions than what we created to promote the operand.
>> +  // This is not profitable.
>> +  if (MatchedSize < SizeWithPromotion)
>> +    return false;
>> +  if (MatchedSize > SizeWithPromotion)
>> +    return true;
>> +  // The promotion is neutral but it may help folding the signextension in
>> +  // loads for instance.
>> +  // Check that we did not create an illegal instruction.
>> +  Instruction *PromotedInst = dyn_cast<Instruction>(PromotedOperand);
>> +  if (!PromotedInst)
>> +    return false;
>> +  return TLI.isOperationLegalOrCustom(PromotedInst->getOpcode(),
>> +                                      EVT::getEVT(PromotedInst->getType()));
> 
> This won't work, because the IR Instructions and SelectionDAG Opcodes
> have different enumeration values.
Right, good catch!

>  The original test case must have
> passed, because it is using a default TargetLowering implementation
> rather than the one from R600.
> 
> I've attached an updated version of the original test case which compiles
> the program using llc and triggers the bug again.
> 
> I couldn't find other transforms that use the isOperation* callbacks, so
> I think you may have to add a function to convert from IR Instruction to
> SelectionDAG Opcodes or implement this some other way.
I think a specific hook in TargetLowering may be the best approach here.
Let me see what I can come up with.
If you have ideas on the naming, let me know :).

Cheers,
Quentin
> 
> Thanks,
> Tom
> <update-cgp-test.case.diff>