[PATCH][FastISel] Let the target decide first if it wants to materialize a constant.

Juergen Ributzka juergen at apple.com
Thu Aug 14 13:11:09 PDT 2014


That is the commit that actually reversed the order for materializing a constant by invoking the target-specific code first. I guess there must be a bug in the AArch64 target-specific materialization code that got exposed by switching the order. Let me see if I can reproduce the error locally and come up with a fix.

-Juergen

On Aug 14, 2014, at 1:05 PM, Chad Rosier <mcrosier at codeaurora.org> wrote:

> The problem child is r215588.
> 
>> Ok, let me revert all changes and see if that fixes it.
>> 
>> -Juergen
>> 
>> On Aug 14, 2014, at 12:47 PM, Chad Rosier <mcrosier at codeaurora.org> wrote:
>> 
>>> I don't think r215597 is the culprit.  All tests previously failing are
>>> continuing to fail.
>>> 
>>>> I know. I have the same issue here with the list too … takes forever
>>>> today.
>>>> 
>>>> -Juergen
>>>> 
>>>> On Aug 14, 2014, at 10:53 AM, Chad Rosier <mcrosier at codeaurora.org>
>>>> wrote:
>>>> 
>>>>> I see the revert (what's up with the slow list today?).  I'll have an
>>>>> answer in a bit.  I need to step out for an hour, so we should know by
>>>>> 1p
>>>>> PT.
>>>>> 
>>>>> Chad
>>>>> 
>>>>> 
>>>>>> Dang, that might have been the other commit I made after this to
>>>>>> extend
>>>>>> the addressing modes for AArch64.
>>>>>> Let me revert that commit first. How long will it take to see the
>>>>>> results
>>>>>> of the revert?
>>>>>> 
>>>>>> Cheers,
>>>>>> Juergen
>>>>>> 
>>>>>> On Aug 14, 2014, at 9:53 AM, Chad Rosier <mcrosier at codeaurora.org>
>>>>>> wrote:
>>>>>> 
>>>>>>> Juergen,
>>>>>>> Also, I'm seeing a large number of spec2K and spec2K6 failures after
>>>>>>> this
>>>>>>> flurry of commits.  I'm not exactly sure which one to blame, but we
>>>>>>> should
>>>>>>> consider reverting to fix the nightlies as well as allow for proper
>>>>>>> review.
>>>>>>> 
>>>>>>> Chad
>>>>>>> 
>>>>>>>> So, this was in my list of things to look at but it seems like you
>>>>>>>> went ahead and committed it without actually waiting for feedback?
>>>>>>>> 
>>>>>>>> -eric
>>>>>>>> 
>>>>>>>> On Tue, Aug 12, 2014 at 9:23 AM, Juergen Ributzka
>>>>>>>> <juergen at apple.com>
>>>>>>>> wrote:
>>>>>>>>> ping :D
>>>>>>>>> 
>>>>>>>>> On Aug 7, 2014, at 2:51 PM, Juergen Ributzka <juergen at apple.com>
>>>>>>>>> wrote:
>>>>>>>>> 
>>>>>>>>>> Hi @ll,
>>>>>>>>>> 
>>>>>>>>>> I want to change the order in which FastISel tries to materialize
>>>>>>>>>> a
>>>>>>>>>> constant.
>>>>>>>>>> 
>>>>>>>>>> Originally it would try to use a simple target-independent
>>>>>>>>>> approach,
>>>>>>>>>> which can lead to the generation of inefficient code.
>>>>>>>>>> 
>>>>>>>>>> On X86 this would result in the use of movabsq to materialize any
>>>>>>>>>> 64bit
>>>>>>>>>> integer constant - even for simple and small values such as 0 and
>>>>>>>>>> 1.
>>>>>>>>>> Also
>>>>>>>>>> some very funny floating-point materialization could be observed
>>>>>>>>>> too.
>>>>>>>>>> 
>>>>>>>>>> On AArch64 it would materialize the constant 0 in a register even
>>>>>>>>>> the
>>>>>>>>>> architecture has an actual "zero" register.
>>>>>>>>>> 
>>>>>>>>>> On ARM it would generate unnecessary mov instructions or not use
>>>>>>>>>> mvn,
>>>>>>>>>> etc
>>>>>>>>>> 
>>>>>>>>>> I want to simply change the order and always asks the target
>>>>>>>>>> first
>>>>>>>>>> if
>>>>>>>>>> it likes to materialize the constant. The patches a pretty simple
>>>>>>>>>> and
>>>>>>>>>> straight forward, but since it changes the way how FastISel works
>>>>>>>>>> with
>>>>>>>>>> respect to constants I wanted everyone affected by it to have at
>>>>>>>>>> least
>>>>>>>>>> a quick look first.
>>>>>>>>>> 
>>>>>>>>>> Thanks
>>>>>>>>>> 
>>>>>>>>>> Cheers,
>>>>>>>>>> Juergen
>>>>>>>>>> 
>>>>>>>>>> <0001-FastISel-AArch64-Cleanup-constant-materialization-co.patch><0002-FastISel-ARM-Fix-a-bug-in-the-integer-materializatio.patch><0003-FastISel-ARM-Use-MOVT-MOVW-if-the-subtarget-requests.patch><0004-FastISel-X86-Refactor-constant-materialization.-NFCI.patch><0005-FastISel-Let-the-target-decide-first-if-it-wants-to-.patch><0006-FastISel-AArch64-Make-use-of-the-zero-register-when-.patch><0007-FastISel-X86-Emit-more-efficient-instructions-for-in.patch><0008-FastISel-X86-Use-XOR-to-materialize-the-0-value.patch>
>>>>>>>>> 
>>>>>>>> 
>>>>>>> 
>>>>>>> 
>>>>>>> --
>>>>>>> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
>>>>>>> hosted by The Linux Foundation
>>>>>>> 
>>>>>> 
>>>>>> 
>>>>> 
>>>>> 
>>>>> --
>>>>> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
>>>>> hosted by The Linux Foundation
>>>>> 
>>>> 
>>>> 
>>> 
>>> 
>>> --
>>> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
>>> hosted by The Linux Foundation
>>> 
>> 
>> 
> 
> 
> -- 
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
> hosted by The Linux Foundation
> 





More information about the llvm-commits mailing list