Hi!

On 12/13/21 6:22 PM, Segher Boessenkool wrote:
> On Mon, Dec 13, 2021 at 02:37:43PM -0600, Bill Schmidt wrote:
>> On 12/13/21 10:54 AM, Segher Boessenkool wrote:
>>> On Mon, Dec 13, 2021 at 11:30:28AM -0500, David Edelsohn wrote:
>>>> On Mon, Dec 13, 2021 at 10:48 AM Bill Schmidt <wschm...@linux.ibm.com> 
>>>> wrote:
>>>>> PR103624 observes that we get segfaults for the 64-bit darn builtins when 
>>>>> compiled
>>>>> on a 32-bit architecture.  The old built-in infrastructure requires 
>>>>> TARGET_64BIT, and
>>>>> this was missed in the new support.  Moving these two builtins from the 
>>>>> [power9]
>>>>> stanza to the [power9-64] stanza solves the problem.
>>>>>
>>>>> Tested the fix on a powerpc-e300c3-linux-gnu cross.  Bootstrapped and 
>>>>> tested on
>>>>> powerpc64le-linux-gnu with no regressions.  Is this okay for trunk?
>>>> Okay.
>>> No, as I said before this is not correct, not without a lot more
>>> explanation at least.  We should not copy errors in the old code into
>>> the new code.  That is negating one of the main advantages of
>>> reimplementing this in the first place!
>> Can you please be more specific?
>>
>> All I have from you before is "It should work for 32-bit though?"  I 
>> responded in the
>> bug report that __builtin_darn_32 was used for this purpose.  I haven't seen 
>> a
>> response to that.  What do you want to see happen?
> That of course does not work for _raw.
>
> These builtins should just return a "long", just like __builtin_ppc_mftb
> does.  All three of them.

Well, that seems wrong for __builtin_darn_32, which maps to an SImode pattern.

So, I assume what you'd like to see is for the other two built-ins to return
long, and for the "&& TARGET_64BIT" to be removed from the darn_raw and darn
patterns?

>
>> The patterns in rs6000.md are darn_32, gated by TARGET_P9_MISC; darn_raw, 
>> gated by
>> TARGET_P9_MISC && TARGET_64BIT; and darn, gated by TARGET_P9_MISC && 
>> TARGET_64BIT.
>> The builtins correspond to these patterns in the obvious way.
>>
>> If you think that these patterns should be enabled differently, that's fine, 
>> but
>> that's a completely different patch than fixing the incorrect built-ins to 
>> match
>> what the patterns do and thus avoid ICEing.
> Avoiding ICEs should not be a goal.  It should be a side effect of doing
> the right thing in the first place!


There's no reason to get snippy.  Given that you approved Kelvin's original
implementation of the darn patterns and built-in functions, I think I can be
forgiven for thinking that those were the desired semantics. :-)

Thanks,
Bill

>
>
> Segher

Reply via email to