On Jun 12, 2024, at 15:00, Richard Biener <rguent...@suse.de> wrote:
>
> On Wed, 12 Jun 2024, René Rebe wrote:
>
>> Hi,
>>
>>> On Jun 12, 2024, at 13:01, Richard Biener <rguent...@suse.de> wrote:
>>>
>>> On Wed, 12 Jun 2024, Rene Rebe wrote:
>>>>
>>>> gcc/
>>>> * config/ia64/ia64.cc: Enable LRA for ia64.
>>>> * config/ia64/ia64.md: Likewise.
>>>> * config/ia64/predicates.md: Likewise.
>>>
>>> That looks simple enough. I cannot find any copyright assignment on
>>> file with the FSF so you probably want to contribute to GCC under
>>> the DCO (see https://gcc.gnu.org/dco.html), in that case please post
>>> patches with Signed-off-by: tags.
>>
>> If it helps for the future, I can apply for copyright assignment, too.
>
> It's not a requirement - you as contributor get the choice under
> which legal framework you contribute to GCC, for the DCO there's
> the formal requirement of Signed-off-by: tags.
>
>>> For this patch please state how you tested it, I assume you
>>> bootstrapped GCC natively on ia64-linux and ran the testsuite.
>>> I can find two gcc-testresult postings, one appearantly with LRA
>>> and one without? Both from May:
>>>
>>> https://sourceware.org/pipermail/gcc-testresults/2024-May/816422.html
>>> https://sourceware.org/pipermail/gcc-testresults/2024-May/816346.html
>>
>> Yes, that are the two I quoted in the patch cover letter.
>>
>> https://gcc.gnu.org/pipermail/gcc-patches/2024-June/654321.html
>>
>>> somehow for example libstdc++ summaries were not merged, it might
>>> be you do not have recent python installed on the system? Or you
>>> didn't use contrib/test_summary to create those mails. It would be
>>> nice to see the difference between LRA and not LRA in the testresults,
>>> can you quote that?
>>
>> We usually cross-compile gcc, but also ran natively for the testsuite.
>> Given the tests run quite long natively on the hardware we currently
>> have, I summed the results them up in the cover letter. I would assume
>> that shoudl be enough to include with a note the resulting kernel and
>> user-space world was booted and worked without issues?
>
> I specifically wondered if bootstrap with LRA enabled succeeds.
> That needs either native or emulated hardware. I think we consider
> ia64-linux a host platform and not only a cross compiler target.
With “also ran” I meant to say we did both, our T2 framework usually
boot-straps everything cross compiled, but also supports native in-system
compilation. Frank also manually natively bootstrapped gcc and ran the
testsuite natively on ia64.
>> If so, I’ll just resend with the additional information added.
>
> For the LRA enablement patch the requirement is that patches should
> state how they were tested - usually you'll see sth like
>
> Boostrapped and tested on x86_64-unknown-linux-gnu.
>
> In your case it was
>
> Cross-built from x86_64-linux(?) to ia64-linux, natively tested
OK - I include the details in v2.
> not sure how you exactly did this though? I've never tried
> testing of a canadian-cross tree - did you copy the whole build
> tree over from the x86 to the ia64 machine?
IIRC the testsuite did not just work copying the canadian-cross.
I did run the testsuite from the cross-compiled gcc using a ssh
based dejagnu board config, but Frank also did fully bootstrap and
ran the testsuite natively.
Thanks,
René
> Thanks,
> Richard.
>
>> Thank you so much,
>> René
>>
>>> Thanks,
>>> Richard.
>>>
>>>> ---
>>>> gcc/config/ia64/ia64.cc | 7 ++-----
>>>> gcc/config/ia64/ia64.md | 4 ++--
>>>> gcc/config/ia64/predicates.md | 2 +-
>>>> 3 files changed, 5 insertions(+), 8 deletions(-)
>>>>
>>>> diff --git a/gcc/config/ia64/ia64.cc b/gcc/config/ia64/ia64.cc
>>>> index ac3d56073ac..d189bfb2cb4 100644
>>>> --- a/gcc/config/ia64/ia64.cc
>>>> +++ b/gcc/config/ia64/ia64.cc
>>>> @@ -618,9 +618,6 @@ static const scoped_attribute_specs *const
>>>> ia64_attribute_table[] =
>>>> #undef TARGET_LEGITIMATE_ADDRESS_P
>>>> #define TARGET_LEGITIMATE_ADDRESS_P ia64_legitimate_address_p
>>>>
>>>> -#undef TARGET_LRA_P
>>>> -#define TARGET_LRA_P hook_bool_void_false
>>>> -
>>>> #undef TARGET_CANNOT_FORCE_CONST_MEM
>>>> #define TARGET_CANNOT_FORCE_CONST_MEM ia64_cannot_force_const_mem
>>>>
>>>> @@ -1329,7 +1326,7 @@ ia64_expand_move (rtx op0, rtx op1)
>>>> {
>>>> machine_mode mode = GET_MODE (op0);
>>>>
>>>> - if (!reload_in_progress && !reload_completed && !ia64_move_ok (op0,
>>>> op1))
>>>> + if (!lra_in_progress && !reload_completed && !ia64_move_ok (op0, op1))
>>>> op1 = force_reg (mode, op1);
>>>>
>>>> if ((mode == Pmode || mode == ptr_mode) && symbolic_operand (op1,
>>>> VOIDmode))
>>>> @@ -1776,7 +1773,7 @@ ia64_expand_movxf_movrf (machine_mode mode, rtx
>>>> operands[])
>>>> }
>>>> }
>>>>
>>>> - if (!reload_in_progress && !reload_completed)
>>>> + if (!lra_in_progress && !reload_completed)
>>>> {
>>>> operands[1] = spill_xfmode_rfmode_operand (operands[1], 0, mode);
>>>>
>>>> diff --git a/gcc/config/ia64/ia64.md b/gcc/config/ia64/ia64.md
>>>> index 698e302081e..d485acc0ea8 100644
>>>> --- a/gcc/config/ia64/ia64.md
>>>> +++ b/gcc/config/ia64/ia64.md
>>>> @@ -2318,7 +2318,7 @@
>>>> (match_operand:DI 3 "register_operand" "f"))
>>>> (match_operand:DI 4 "nonmemory_operand" "rI")))
>>>> (clobber (match_scratch:DI 5 "=f"))]
>>>> - "reload_in_progress"
>>>> + "lra_in_progress"
>>>> "#"
>>>> [(set_attr "itanium_class" "unknown")])
>>>>
>>>> @@ -3407,7 +3407,7 @@
>>>> (match_operand:DI 2 "shladd_operand" "n"))
>>>> (match_operand:DI 3 "nonmemory_operand" "r"))
>>>> (match_operand:DI 4 "nonmemory_operand" "rI")))]
>>>> - "reload_in_progress"
>>>> + "lra_in_progress"
>>>> "* gcc_unreachable ();"
>>>> "reload_completed"
>>>> [(set (match_dup 0) (plus:DI (mult:DI (match_dup 1) (match_dup 2))
>>>> diff --git a/gcc/config/ia64/predicates.md b/gcc/config/ia64/predicates.md
>>>> index 01a4effd339..85f5380e734 100644
>>>> --- a/gcc/config/ia64/predicates.md
>>>> +++ b/gcc/config/ia64/predicates.md
>>>> @@ -347,7 +347,7 @@
>>>> allows reload the opportunity to avoid spilling addresses to
>>>> the stack, and instead simply substitute in the value from a
>>>> REG_EQUIV. We'll split this up again when splitting the insn. */
>>>> - if (reload_in_progress || reload_completed)
>>>> + if (lra_in_progress || reload_completed)
>>>> return true;
>>>>
>>>> /* Some symbol types we allow to use with any offset. */
>>>>
>>>
>>> --
>>> Richard Biener <rguent...@suse.de>
>>> SUSE Software Solutions Germany GmbH,
>>> Frankenstrasse 146, 90461 Nuernberg, Germany;
>>> GF: Ivo Totev, Andrew McDonald, Werner Knoblich; (HRB 36809, AG Nuernberg)
>>
>>
>
> --
> Richard Biener <rguent...@suse.de>
> SUSE Software Solutions Germany GmbH,
> Frankenstrasse 146, 90461 Nuernberg, Germany;
> GF: Ivo Totev, Andrew McDonald, Werner Knoblich; (HRB 36809, AG Nuernberg)
--
ExactCODE GmbH, Lietzenburger Str. 42, DE-10789 Berlin
http://exactcode.com | http://exactscan.com | http://ocrkit.com
