Hi,
Segher Boessenkool <seg...@kernel.crashing.org> writes:
> On Tue, May 14, 2024 at 11:00:38AM +0800, Jiufu Guo wrote:
>> >> diff --git a/gcc/config/rs6000/rs6000.cc b/gcc/config/rs6000/rs6000.cc
>> >> index 117999613d8..50943d76f79 100644
>> >> --- a/gcc/config/rs6000/rs6000.cc
>> >> +++ b/gcc/config/rs6000/rs6000.cc
>> >> @@ -14659,6 +14659,12 @@ print_operand_address (FILE *file, rtx x)
>> >> else if (SYMBOL_REF_P (x) || GET_CODE (x) == CONST
>> >> || GET_CODE (x) == LABEL_REF)
>> >> {
>> >> + if (this_is_asm_operands && !address_operand (x, VOIDmode))
>> >
>> > Do we really need this_is_asm_operands here?
>> I understand your point:
>> since in function 'print_operand_address' which supports not only user
>> asm code. So, it maybe incorrect if 'x' is not an 'address_operand',
>> no matter this_is_asm_operands.
>>
>> Here, 'this_is_asm_operands' is needed because it would be treated as an
>> user fault in asm-code (otherwise, internal_error in the compiler).
>
> You almost never want to test for asm, and just give the same error you
> would give in non-asm. It is the same problem after all, and giving the
> user the same error message is the most helpful thing to do!
Yes, just as Kewen's comments. The testing on 'this_is_asm_operands' and
'address_operand' is not in good place.
The message emitting and it's checking chould be more straightforward,
something like:
/* emit error for user asm code, or fault in compiler. */
else if (TARGET_TOC)
output_operand_lossage ("xxx");
I would update the patch for this.
BR,
Jeff(Jiufu) Guo
>
> It can be useful to not say "ICE", but it already is prevented from
> doing that here.
>
>
> Segher
