RE: [PATCH v1 1/1] gas: expr: fix support .long 0U and .long 0u

["Frager, Neal" <neal.frager@amd.com>]

>> Fix support for .long 0U and .long 0u in GCC.
>>
>> This patch has been tested for years of AMD Xilinx Yocto releases as 
>> part of the following patch set:
>>
>> https://github.com/Xilinx/meta-xilinx/tree/master/meta-microblaze/rec
>> ipes-devtools/binutils/binutils
>>
>> Signed-off-by: Neal Frager <neal.frager@amd.com>
>> ---
>>   gas/expr.c | 9 +++++++++
>>   1 file changed, 9 insertions(+)
> 
> Without a testcase demonstrating what's wrong, I'd almost be inclined 
> to say that ...
> 
>> --- a/gas/expr.c
>> +++ b/gas/expr.c
>> @@ -824,6 +824,15 @@ operand (expressionS *expressionP, enum expr_mode mode)
>>   	      break;
>>   	    }
>>   	}
>> +      if ((*input_line_pointer == 'U') || (*input_line_pointer == 'u'))
>> +	{
>> +	  input_line_pointer--;
>> +
>> +	  integer_constant ((NUMBERS_WITH_SUFFIX || flag_m68k_mri)
>> +	                    ? 0 : 10,
>> +	                    expressionP);
>> +	  break;
>> +	}
>>         c = *input_line_pointer;
>>         switch (c)
>>   	{
> 
> ... this ought to be covered by logic in integer_constant() already. 
> But I think I see what the issue is. Nevertheless, go look for 
> tc_allow_U_suffix, which wants using here as well. Further I think the 
> same issue then exists for L/l suffixes?
> 
> Plus I think you want to add the new code to the switch() statement 
> rather than immediately ahead of it.
> 
> Jan
> 

Hi Michael, Jan,

> Neal --

> Can you provide a test case for this patch?  I'm not able to reproduce any error.

>Please take a look at gas/expr.c:541-543 (PR 19910: Look for, and ignore, a U suffix to the number).  Is this a fix for the same issue?
> https://sourceware.org/bugzilla/show_bug.cgi?id=19910

>As Jan says, it looks like this code, if needed, should be in the switch
>  statement.

Thank you for your review.

I would just like to clarify where this patch is coming from.  
I am not actually the author of the patches I am submitting.
When we provide a Microblaze toolchain with PetaLinux, Yocto or Vitis, we are actually applying all of the following patches to the binutils portion of the toolchain:
https://github.com/Xilinx/meta-xilinx/tree/master/meta-microblaze/recipes-devtools/binutils/binutils

Instead of continuing to carry the maintenance of this patch set with all of our releases, I would like to get as many of these patches upstreamed as possible.

This particular patch is coming from here:
https://github.com/Xilinx/meta-xilinx/blob/master/meta-microblaze/recipes-devtools/binutils/binutils/0009-Patch-Microblaze-fixed-bug-in-GCC-so-that-It-will-su.patch

It is possible that this patch is not required anymore.  I will check internally at AMD Xilinx to see if I can identify a way to reproduce the error that this patch is solving.

Until I can prove that this patch is still needed, let's skip it.  If it is really no longer necessary, I will try to get it removed from our list of patches.

Thanks for your help!

Best regards,
Neal Frager
AMD