Richard Sandiford via Gcc-patches <gcc-patches@gcc.gnu.org> writes:
> Tamar Christina <tamar.christ...@arm.com> writes:
>>> -----Original Message-----
>>> From: Richard Sandiford <richard.sandif...@arm.com>
>>> Sent: Monday, May 16, 2022 12:36 PM
>>> To: Tamar Christina <tamar.christ...@arm.com>
>>> Cc: gcc-patches@gcc.gnu.org; nd <n...@arm.com>; rguent...@suse.de;
>>> jeffreya...@gmail.com
>>> Subject: Re: [PATCH 1/3]middle-end: Add the ability to let the target decide
>>> the method of argument promotions.
>>>
>>> Tamar Christina <tamar.christ...@arm.com> writes:
>>> > Hi All,
>>> >
>>> > Some targets require function parameters to be promoted to a different
>>> > type on expand time because the target may not have native
>>> > instructions to work on such types. As an example the AArch64 port
>>> > does not have native instructions working on integer 8- or 16-bit
>>> > values. As such it promotes every parameter of these types to 32-bits.
>>>
>>> This doesn't seem specific to parameters though. It applies to any
>>> 8- or 16-bit variable. E.g.:
>>>
>>> #include <stdint.h>
>>> uint8_t foo(uint32_t x, uint32_t y) {
>>> uint8_t z = x != 0 ? x : y;
>>> return z + 1;
>>> }
>>>
>>> generates:
>>>
>>> foo:
>>> cmp w0, 0
>>> and w1, w1, 255
>>> and w0, w0, 255
>>> csel w0, w1, w0, eq
>>> add w0, w0, 1
>>> ret
>>>
>>> So I think the new behaviour is really a modification of the PROMOTE_MODE
>>> behaviour rather than the PROMOTE_FUNCTION_MODE behaviour.
>>>
>>> FWIW, I agree with Richard that it would be better not to add a new hook.
>>> I think we're really making PROMOTE_MODE choose between
>>> SIGN_EXTEND, ZERO_EXTEND or SUBREG (what LLVM would call “any
>>> extend”) rather than the current SIGN_EXTEND vs. ZERO_EXTEND choice.
>>
>> Ah, I hadn't realized this also applied to locals.. ok I can modify
>> PROMOTE_MODE then,
>> but I also need the actual SSA_NAME and not just the type so will have to
>> pass this along.
>>
>> From a practical point of view.. the actual hook however is implemented by
>> 34 targets,
>> would I need to CC maintainers for each of them or would global maintainer
>> approval
>> suffice for these mostly mechanical changes?
>
> Yeah, single approval should be enough mechanical changes. It would be
> good to do the interface change and mechanical target changes as a
> separate prepatch if possible though.
>
> I'm not sure about passing the SSA name to the target though, or the
> way that the aarch64 hook uses the info. It looks like a single cold
> comparison could defeat the optimisation for hot code.
>
> If we do try to make the decision based on uses at expand time, it might
> be better for the analysis to be in target-independent code, with help
> from the target to decide where extensions are cheap. It still feels
> a bit hacky though.
>
> What stops us from forming cbz/cbnz when the extension is done close to
> the comparison (from the comment in 2/3)? If we can solve that, could
> we simply do an any-extend all the time, and treat removing redundant
> extensions as a global availability problem?
(which would run after combine)
>
> What kind of code do we emit when do an extension just before
> an operation? If the DECL_RTL is (subreg:QI (reg:SI R) 0), say,
> then it should be safe to do the extension directly into R:
>
> (set (reg:SI X) (zero_extend:SI (subreg:QI (reg:SI X))))
Oops, that should of course be:
(set (reg:SI R) (zero_extend:SI (subreg:QI (reg:SI R))))
> which avoids the problem of having two values live at once
> (the zero-extended value and the any-extended value).
>
> Having identical instructions for each extension would also make
> it easier for any global pass to move them and remove redundancies.
>
> Thanks,
> Richard
