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

[Quentin Colombet]

Hi Tom,

That’s weird that bugpoint is linking correctly for you.

I have dug in that linking problem and here is what I found.
Because of the CodeGenPrepare pass, libLLVMScalarOpts has a dependency on libLLVMCodeGen, which is not included on the link command of bugpoint.

So far it was working because all the functions of TargetLowering called in CodeGenPrepare are either:
- virtual function (resolved dynamically).
- inlined in the header (code available statically).

InstructionOpcodeToISD is not a virtual function and not inlined in the header, thus the linking problem.

We could add a dependency in bugpoint on libLLVMCodeGen, but I am not sure we want to do that.
Another possibility would be to inline InstructionOpcodeToISD into the header, though I haven’t check how much this is pulling into the header.

Thoughts?

Cheers,
-Quentin

On Feb 18, 2014, at 2:28 PM, Tom Stellard <tom at stellard.net> wrote:

> On Tue, Feb 18, 2014 at 12:45:33PM -0800, Quentin Colombet wrote:
>> 
>> On Feb 18, 2014, at 11:19 AM, Tom Stellard <tom at stellard.net> wrote:
>> 
>>> 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?
>> Makefile on Mac OS.
>> 
>>> Did you try running make clean first?
>> I’ve tried but it does not seem to make the trick.
>> 
>> Could you try the attached patch?
>> 
> 
> I've tested this and it builds fine for me on Linux with autoconf
> and it also fixes the testcase.
> 
> -Tom
> 
>> Thanks,
>> -Quentin
>> 
>>> 
>>> -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

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