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

[Tom Stellard]

On Tue, Feb 18, 2014 at 11:00:05AM -0800, Quentin Colombet wrote:
> 
> On Feb 18, 2014, at 10:30 AM, Tom Stellard <tom at stellard.net> wrote:
> 
> > On Tue, Feb 18, 2014 at 09:47:21AM -0800, Quentin Colombet wrote:
> >> 
> >> On Feb 18, 2014, at 9:37 AM, Quentin Colombet <qcolombet at apple.com> wrote:
> >> 
> >>> 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 :).
> >> In fact, we may be able to use this:
> >> int TargetLoweringBase::InstructionOpcodeToISD(unsigned Opcode)const
> > 
> > Oh, I didn't see that.  That should work.
> Almost there.
> I am seeing a link error when creating the bugpoint executable.
> 
> llvm[2]: Linking Release executable bugpoint (without symbols)
> Undefined symbols for architecture x86_64:
>   "llvm::TargetLoweringBase::InstructionOpcodeToISD(unsigned int) const", referenced from:
>       (anonymous namespace)::AddressingModeMatcher::MatchOperationAddr(llvm::User*, unsigned int, unsigned int, bool*) in libLLVMScalarOpts.a(CodeGenPrepare.o)
> ld: symbol(s) not found for architecture x86_64
> 
> The thing that I do not understand is why the linker is not find this function, whereas it does not complain about TargetLoweringBase::isLegalAddressingMode.
> I may have missed something obvious, but I do not see it.

I'm not seeing this error on my machine.  What build system are you
using?  Did you try running make clean first?

-Tom

> 
> Any idea?
> 
> Thanks,
> -Quentin
> > 
> > -Tom
> >> 
> >>> 
> >>> Cheers,
> >>> Quentin
> >>>> 
> >>>> Thanks,
> >>>> Tom
> >>>> <update-cgp-test.case.diff>
> >>> 
> >>> _______________________________________________
> >>> llvm-commits mailing list
> >>> llvm-commits at cs.uiuc.edu
> >>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>