[patch,testsuite] Support dg-require-effective-target label_offsets.

[Georg-Johann Lay]

On 04.11.2016 03:48, Mike Stump wrote:
> On Nov 3, 2016, at 8:25 AM, Georg-Johann Lay <avr@gjlay.de> wrote:
>>
>> On 28.10.2016 17:58, Mike Stump wrote:
>>> On Oct 27, 2016, at 3:16 AM, Georg-Johann Lay <avr@gjlay.de> wrote:
>>>>
>>>> Now imagine some arithmetic like &&LAB2 - &&LAB1.  This might result in
>>>> one or two stub addresses, and difference between such addresses is a
>>>> complete different thing than the difference between the original labels:
>>>> The result might differ in absolute value and in sign, i.e. you can't even
>>>> determine whether LAB1 or LAB2 comes first in the generated code as the
>>>> order of stubs might differ from the order of respective labels.
>>>
>>> So, I think this all doesn't matter any.  Every address gs(LAB) fits in
>>> 16-bits by definition, and every gs(LAB1) - gs(LAB2) fits into 16 bits and
>>
>> Yes, you are right.  Label differences could be implemented.  AFAIK there is currently no activity by the Binutils folks to add respective support, so it is somewhat pointless to add that support to avr-gcc...
>
>
> [ pause, built an avr compiler ] So, I checked code-gen for gcc.c-torture/compile/labels-2.c and it looked fine:
>
>         ldi r24,lo8(gs(.L2))
>         ldi r25,hi8(gs(.L2))
>         std Y+2,r25
>         std Y+1,r24
>         ldi r18,lo8(gs(.L3))
>         ldi r19,hi8(gs(.L3))
>         ldi r24,lo8(gs(.L4))
>         ldi r25,hi8(gs(.L4))
>         mov r20,r18
>         mov r21,r19
>         sub r20,r24
>
> So, apparently quite a lot of the code-gen is already wired up.  Since this generated code, I wasn't sure what error you're trying to hide?  I tried all the test cases your mentioned, none failed to produce code or failed to assemble that code with -mmcu=atmega2560 nor any other cpu I could find, so, trivially, I must not quite understand what doesn't work yet.
>
> I did notice that gcc.c-torture/execute/pr70460.c produced:
>
> .word   .L3-(.L2)
>
> which seems wrong, given all the other code-gen.  I think this has to be gs(.L3)-(gs(.L2)), no?  I tried to get binutils to do that for me directly with a .word and it seemed resistant; which is unfortunate.

Well, the GCC specification on void* arithmetic requires that void is handled 
as if it has a size of one, and .L3-.L2 meets that requirement.  As code 
addresses on avr are word addresses, not byte addresses, the above code will 
crash.  A working expression would be (.L3-.L2)/2 but 1) this conflicts with 
the gcc specification 2) it does not work with stubs 3) the linker does not 
handle this correctly if relaxation is on.

All the label-arithmetic test cases in GCC testsuite don't actually need stub 
generation because the binaries are not big enough.  Using gs, the expression 
would be gs(.L3)-(gs(.L2)) as gs implements word addresses, not byte addresses.

This also means the problem is not only present with 3-byte PC devises but also 
for smaller ones.

>
> If you consider the above to be a code-gen bug, then you can do something like:
>
> Index: config/avr/avr.c
> ===================================================================
> --- config/avr/avr.c    (revision 241837)
> +++ config/avr/avr.c    (working copy)
> @@ -9422,6 +9422,21 @@
>        x = plus_constant (Pmode, x, AVR_TINY_PM_OFFSET);
>      }
>
> +  /* We generate stubs using gs(), but binutils doesn't support that
> +     here.  */
> +  if (AVR_3_BYTE_PC
> +      && GET_CODE (x) == MINUS
> +      && GET_CODE (XEXP (x, 0)) == LABEL_REF
> +      && GET_CODE (XEXP (x, 1)) == LABEL_REF)
> +    return false;
> +  if (AVR_3_BYTE_PC
> +      && GET_CODE (x) == MINUS
> +      && GET_CODE (XEXP (x, 0)) == SUBREG
> +      && GET_CODE (XEXP (XEXP (x, 0), 0)) == LABEL_REF
> +      && GET_CODE (XEXP (x, 1)) == SUBREG
> +      && GET_CODE (XEXP (XEXP (x, 1), 0)) == LABEL_REF)
> +    return false;
> +
>    return default_assemble_integer (x, size, aligned_p);
>  }
>
> to get the compiler to error out to prevent bad code-gen that binutils can't handle.  If you want, you can also declare by fiat that subtraction might be performed on the stub or the actual function, and so any code that does this isn't supported (runtime unspecified behavior), and then you just mark the execute testcase as bad (not supportable on avr, since avr blew the binutils support for gs).  The compile only test cases work fine now, so nothing needs to be done for them.

ICE is not a nice approach, IMO.  I'd prefer some more graceful error using 
code locations of the labels (which are not available).

>
> If you go for the above and make it a hard error, then a patch in the style of the original patch would be fine, but please just mark the ones that fail and please key of avr*, as this is an avr-only misfeature.  If avr did it right, they would complete gs() support so that gs(LAB)-gs(LAB) works.  gcc has a fetish for + and - working with labels and constants.
>
> The completely different solution would be to never use gs(), and generate code that is 3 byte aware, but, I suspect you're unlikely to want to do that.

That's the worst solution of all because it changes the ABI, leads to code 
bloat, and many more issues.  And just reduce testsuite fallout for some crazy 
tests? Really?