Fine. I withdraw the patch request, and will remove my name from
the bugzilla. Somebody else can deal with it. I have more important
things to worry about.
Bill
On 2/11/22 1:31 AM, Segher Boessenkool wrote:
> Hi!
>
> On Thu, Feb 10, 2022 at 04:28:02PM -0600, Bill Schmidt wrote:
>> On 2/10/22 4:11 PM, Segher Boessenkool wrote:
>>>> No, trunk has this, for example:
>>>>
>>>> const signed int __builtin_altivec_vclzlsbb_v16qi (vsc);
>>>> VCLZLSBB_V16QI vctzlsbb_v16qi {endian}
>>> I see this on trunk:
>>>
>>> const signed int __builtin_altivec_vclzlsbb_v16qi (vsc);
>>> VCLZLSBB_V16QI vclzlsbb_v16qi {}
>>>
>>> Oh, you changed it? Please fix it, then.
>> In a patch you approved, yes.
> Yes, I missed it. That is not an argument that it would be good or
> should not be change.
>
>> I don't really understand why you want
>> it changed now.
> Because it is wrong.
>
>> You must not be looking at the most recent trunk revision.
> Indeed I haven't been able to update master for a week or so, it does
> not bootstrap, as we have talked about.
>
>>>> Throughout the new builtin infrastructure, the defaults are set for
>>>> little-endian, and the "endian" flag changes behavior for big-endian.
>>> That is a big mistake. There are many machine instructions that are
>>> *always* big-endian (most even!), and none that are always
>>> little-endian. So this should be fixed, sooner rather than later :-(
>> That does not seem like a good idea in stage 4 to me. That requires
>> yet another patch to reverse a bunch of other things unnecessarily.
> Things that were added in stage 4, a few days ago even. Things that are
> broken and wrong. Things I do not want to have to release with and deal
> with all the pain of having broken released versions.
>
>> This is a purely arbitrary choice.
> No, it is not. It flies in the face of consistency.
>
>> The endian flag is only used when
>> a built-in function must have one behavior for big-endian, and another
>> behavior for little-endian. Which one is chosen as the default is
>> absolutely arbitrary.
> The one that corresponds to the name should be the default. I don't see
> how you can argue otherwise.
>
>> When we expand the built-in we will either
>> accept the default or change to the other. The existence of machine
>> instructions that are only big-endian has nothing to do with the case;
>> what matters is the existence of built-in functions that have two
>> behaviors.
> Everything in our backend is BE by default, just like everything in the
> architecture is. Yes, LE works almost as well (or just as well) in most
> places, but everything is named assuming BE. This consistency is hugely
> important, without it the reader will not understand things as well and
> as easily.
>
>>>> That's something that should be fixed, I guess, but it's orthogonal
>>>> to this patch.
>>> Fixing it later is more work :-(
>>>
>>> Please at least open a bug report for it.
>> I can do that.
> Thanks!
>
>>> The other things need fixing before the patch is okay.
>> I'd ask you to reconsider, as explained above.
> It is purely an implementation thing, and it is completely trivial to
> do. If you truly are afraid of breaking things (you should not be), it
> is marginally acceptable to do this as the very first thing in stage 1.
>
> Consistency matters. Naming matters. These shape how we think about
> things.
>
>
> Segher
