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

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

[Georg-Johann Lay]

[-- Attachment #1: Type: text/plain, Size: 964 bytes --]

There are targets that support taking values of labels but where any arithmetic 
on such values might produce garbage.

This patch introduces new dg-require-effective-target label_offsets which is a 
subset of label_values, and adjusts respective test cases to the more 
restricted predicate.

Run tests against avr-unknown-none ATmega2560 where it makes actually a 
difference between label_values and label_offsets.

Ok for trunk?

Johann

gcc/testsuite/
	* lib/target-supports.exp (check_effective_target_label_offsets):
	New proc.
	* gcc.dg/20021029-1.c (dg-require-effective-target): Require
	more restrict label_offsets instead of label_values.
	* gcc.dg/pr16973.c: Dito.
	* gcc.dg/torture/pr66123.c: Dito.
	* gcc.dg/torture/pr66178.c: Dito.
	* gcc.c-torture/compile/20021108-1.c: Dito.
	* gcc.c-torture/compile/920501-7.c: Dito.
	* gcc.c-torture/compile/labels-2.c: Dito.
	* gcc.c-torture/compile/labels-3.c: Dito.
	* gcc.c-torture/execute/pr70460.c: Dito.

[-- Attachment #2: label-offsets.diff --]
[-- Type: text/x-patch, Size: 4843 bytes --]

Index: gcc.c-torture/compile/20021108-1.c
===================================================================
--- gcc.c-torture/compile/20021108-1.c	(revision 241546)
+++ gcc.c-torture/compile/20021108-1.c	(working copy)
@@ -1,4 +1,4 @@
-/* { dg-require-effective-target label_values } */
+/* { dg-require-effective-target label_offsets } */
 
 int
 main()
Index: gcc.c-torture/compile/920501-7.c
===================================================================
--- gcc.c-torture/compile/920501-7.c	(revision 241546)
+++ gcc.c-torture/compile/920501-7.c	(working copy)
@@ -1,3 +1,3 @@
-/* { dg-require-effective-target label_values } */
+/* { dg-require-effective-target label_offsets } */
 
 x(){if(&&e-&&b<0)x();b:goto*&&b;e:;}
Index: gcc.c-torture/compile/labels-2.c
===================================================================
--- gcc.c-torture/compile/labels-2.c	(revision 241546)
+++ gcc.c-torture/compile/labels-2.c	(working copy)
@@ -1,4 +1,4 @@
-/* { dg-require-effective-target label_values } */
+/* { dg-require-effective-target label_offsets } */
 
 struct bp { void *v, *b, *e; };
 f ()
Index: gcc.c-torture/compile/labels-3.c
===================================================================
--- gcc.c-torture/compile/labels-3.c	(revision 241546)
+++ gcc.c-torture/compile/labels-3.c	(working copy)
@@ -1,6 +1,6 @@
 /* Verify that we can narrow the storage associated with label diffs.  */
 /* { dg-require-effective-target indirect_jumps } */
-/* { dg-require-effective-target label_values } */
+/* { dg-require-effective-target label_offsets } */
 
 int foo (int a)
 {
Index: gcc.c-torture/execute/pr70460.c
===================================================================
--- gcc.c-torture/execute/pr70460.c	(revision 241546)
+++ gcc.c-torture/execute/pr70460.c	(working copy)
@@ -1,5 +1,5 @@
 /* { dg-require-effective-target indirect_jumps } */
-/* { dg-require-effective-target label_values } */
+/* { dg-require-effective-target label_offsets } */
 
 /* PR rtl-optimization/70460 */
 
Index: gcc.dg/20021029-1.c
===================================================================
--- gcc.dg/20021029-1.c	(revision 241546)
+++ gcc.dg/20021029-1.c	(working copy)
@@ -3,7 +3,7 @@
 /* { dg-do compile { target fpic } } */
 /* { dg-options "-O2 -fpic" } */
 /* { dg-final { scan-assembler-not ".data.rel.ro.local" } } */
-/* { dg-require-effective-target label_values } */
+/* { dg-require-effective-target label_offsets } */
 /* { dg-require-effective-target indirect_jumps } */
 
 int foo (int a)
Index: gcc.dg/pr16973.c
===================================================================
--- gcc.dg/pr16973.c	(revision 241546)
+++ gcc.dg/pr16973.c	(working copy)
@@ -3,7 +3,7 @@
    to add back the label.   */
 
 /* { dg-options "" } */
-/* { dg-require-effective-target label_values } */
+/* { dg-require-effective-target label_offsets } */
 
 void
 f (void)
Index: gcc.dg/torture/pr66123.c
===================================================================
--- gcc.dg/torture/pr66123.c	(revision 241546)
+++ gcc.dg/torture/pr66123.c	(working copy)
@@ -1,5 +1,5 @@
 /* { dg-do compile } */
-/* { dg-require-effective-target label_values } */
+/* { dg-require-effective-target label_offsets } */
 
 int
 test (int foo)
Index: gcc.dg/torture/pr66178.c
===================================================================
--- gcc.dg/torture/pr66178.c	(revision 241546)
+++ gcc.dg/torture/pr66178.c	(working copy)
@@ -1,5 +1,5 @@
 /* { dg-do compile } */
-/* { dg-require-effective-target label_values } */
+/* { dg-require-effective-target label_offsets } */
 
 int test(void)
 {
Index: lib/target-supports.exp
===================================================================
--- lib/target-supports.exp	(revision 241546)
+++ lib/target-supports.exp	(working copy)
@@ -740,6 +740,31 @@ proc check_effective_target_label_values
     }]
 }
 
+# Return 1 if offsetting label values is supported, 0 otherwise.
+# A typical offset is the value of a different label like in
+# &&lab1 - &&lab2.
+
+proc check_effective_target_label_offsets {} {
+    # Offsetting labels implies we can take values of labels.
+    if { ![check_effective_target_label_values] } {
+	return 0;
+    }
+    if { [istarget avr-*-*] } {
+	# If the value of a label does not fit into 16 bits, the linker
+	# will generate a stub (containing a direct jump) and we end up
+	# with the address of the stub instead of the address of the very
+	# label.  Whereas it is legitimate to use such addresses for
+	# indirect jumps, it makes no sense to perform any arithmetic
+	# on such addresses.
+	return [check_no_compiler_messages label_offsets assembly {
+	    #ifdef __AVR_3_BYTE_PC__
+	    #error NO
+	    #endif
+	}]
+    }
+    return 1;
+}
+
 # Return 1 if builtin_return_address and builtin_frame_address are
 # supported, 0 otherwise.
 

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

[Bernd Schmidt]

On 10/26/2016 04:46 PM, Georg-Johann Lay wrote:
> +    if { [istarget avr-*-*] } {
> +	# If the value of a label does not fit into 16 bits, the linker
> +	# will generate a stub (containing a direct jump) and we end up
> +	# with the address of the stub instead of the address of the very
> +	# label.  Whereas it is legitimate to use such addresses for
> +	# indirect jumps, it makes no sense to perform any arithmetic
> +	# on such addresses.
> +	return [check_no_compiler_messages label_offsets assembly {
> +	    #ifdef __AVR_3_BYTE_PC__
> +	    #error NO
> +	    #endif
> +	}]
> +    }
> +    return 1;

I'm not sure I understand the failure mode. Sure, you're not getting the 
address of the actual label, but the address of one that's equivalent - 
so why can't you do arithmetic on it? Where does it go wrong?

Am I right in thinking that only the execution test actually fails?


Bernd

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

[Georg-Johann Lay]

On 26.10.2016 18:51, Bernd Schmidt wrote:
> On 10/26/2016 04:46 PM, Georg-Johann Lay wrote:
>> +    if { [istarget avr-*-*] } {
>> +    # If the value of a label does not fit into 16 bits, the linker
>> +    # will generate a stub (containing a direct jump) and we end up
>> +    # with the address of the stub instead of the address of the very
>> +    # label.  Whereas it is legitimate to use such addresses for
>> +    # indirect jumps, it makes no sense to perform any arithmetic
>> +    # on such addresses.
>> +    return [check_no_compiler_messages label_offsets assembly {
>> +        #ifdef __AVR_3_BYTE_PC__
>> +        #error NO
>> +        #endif
>> +    }]
>> +    }
>> +    return 1;
>
> I'm not sure I understand the failure mode. Sure, you're not getting the
> address of the actual label, but the address of one that's equivalent - so why
> can't you do arithmetic on it? Where does it go wrong?

There are devices where the address of a label does not fit into 16 bits, which 
is a problem for indirect calls because not all of the program memory can be 
targeted by 16 bits.  The solution was to emit taking address of label LAB as 
gs(LAB) and let the linker resolve this ("gs" stands for "generate stub").

If LAB actually fits in 16 bits nothing special happens and we have LAB = gs(LAB).

If LAB doesn't fit then the linker generates a stub like:

gs_LAB:
    jump LAB

and resolves gs_LAB = gs(LAB).  gs_LAB must be located in the lower part of the 
program memory so that gs_LAB will fit into 16 bits.

An indirect call / jump then is 1) take address of LAB which returns gs_LAB and 
2) jump or call that address which jumps or calls gs_LAB which then jumps to LAB.

This works because absolute jumps and calls can target all of the program 
memory.  Moreover comparing gs() labels against each other or against 0 for 
(un)equality will also work as expected.

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.

> Am I right in thinking that only the execution test actually fails?
>
> Bernd

Some of the run tests are failing, but not all of them.  This is because gs() 
depends on code size and location.  But there are also some compile tests that 
fail as avr_print_operand_address tries to warn about such code with "pointer 
offset from symbol maybe incorrect".

Johann

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

[Bernd Schmidt]

On 10/27/2016 12:16 PM, Georg-Johann Lay 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.

Ok, so you can't expect to use the value directly, but I think this is 
not too different from other targets. You still should be able to add 
the difference back to &&LAB1 and expect to get &&LAB2 or gs(&&LAB2). 
That's what the execution test does - why isn't this working?


Bernd

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

[Georg-Johann Lay]

On 27.10.2016 12:49, Bernd Schmidt wrote:
> On 10/27/2016 12:16 PM, Georg-Johann Lay 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.
>
> Ok, so you can't expect to use the value directly, but I think this is not too
> different from other targets. You still should be able to add the difference
> back to &&LAB1 and expect to get &&LAB2 or gs(&&LAB2). That's what the
> execution test does - why isn't this working?
>
> Bernd

Makes sense what you are writing...  Just replacing a label should not do any 
harm (except for ordering).  I had a deeper look into it and some tests are 
failing because of a missing Binutils support for special expressions, not 
because the tests don't work per se.  Patching the generated asm to results as 
would be provided by working Binutils make the test pass.

Johann

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

[Mike Stump]

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 thus is valid for all 16-bit one contexts.  The fact the order between the stub and the actual code is different is irrelevant, it is a private implementation detail of the port, the point is the semantics are fixed and constant and useful.  In deed that there is even a stub is a private implementation detail of the port.  I think the `extra' helpful warning from avr_print_operand_address is wrong and should be removed.  Think of the label as gs(LAB), not LAB, burn LAB from your mind.  Once you do that, you see you can't talk about the order LAB1 > LAB2, the concept doesn't make any sense.  The _only_ think you can talk about is gs(LAB1) > gs(LAB2).  And because of that, it is always consistent and works just fine.

Once that misguided complains from gcc and bisutils are fixed, are their any failing cases?

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

[Georg-Johann Lay]

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...

Bottom line is that label offsets are not supported by avr BE, and the 
intention of the patch is to reduce FAIL noise in testsuite results because of 
the missing support.

If a dg-require is not appropriate, should I rather add dg-skip-if to every 
test case?  I'd still prefer the dg-require solution because if the Binutils 
will ever support label differences, then the testsuite will automatically 
catch up with that.

> thus is valid for all 16-bit one contexts.  The fact the order between the
> stub and the actual code is different is irrelevant, it is a private

Only if the code is not executed; there are several test cases that compute 
relative placements of labels like:

x(){if(&&e-&&b<0)x();b:goto*&&b;e:;}

If a test ever relies on the fact that &&e - &&b tells anything about the 
ordering of the labels, the code is about to crash.

> implementation detail of the port, the point is the semantics are fixed and
> constant and useful.  In deed that there is even a stub is a private
> implementation detail of the port.  I think the `extra' helpful warning from
> avr_print_operand_address is wrong and should be removed.  Think of the

The following code simple makes absolutely no sense in the presence of stubs:
int
test (int foo)
{
   static void *dummy[] = { &&a, &&b };
   goto *((char *) &&b - 2 * (foo < 0));
a:
b:
   return 0;
}

It's only a compile test (the original PR66123 is about ICE), but the compiler 
cannot know that it's just a crazy test case that won't be executed...

When you are offsetting from a stub you might end up *anywhere*, even in some 
data range.

> label as gs(LAB), not LAB, burn LAB from your mind.  Once you do that, you
> see you can't talk about the order LAB1 > LAB2, the concept doesn't make any
> sense.  The _only_ think you can talk about is gs(LAB1) > gs(LAB2).  And
> because of that, it is always consistent and works just fine.
>
> Once that misguided complains from gcc and bisutils are fixed, are their any
> failing cases?

State of matters is that Binutils support is missing, and if I understand you 
correctly, dg-require is not appropriate to factor out availability of such 
features?

Johann

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

[Mike Stump]

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.

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.

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.

> Bottom line is that label offsets are not supported by avr BE, and the intention of the patch is to reduce FAIL noise in testsuite results because of the missing support.

So, what doesn't work?  I tried all the tests cases on a top of tree compiler, and nothing I did ever failed.

>> thus is valid for all 16-bit one contexts.  The fact the order between the
>> stub and the actual code is different is irrelevant, it is a private
> 
> Only if the code is not executed; there are several test cases that compute relative placements of labels like:
> 
> x(){if(&&e-&&b<0)x();b:goto*&&b;e:;}
> 
> If a test ever relies on the fact that &&e - &&b tells anything about the ordering of the labels, the code is about to crash.

No, I checked the code, it's perfectly fine, with the one exception that I think .L3 is used in places where gs(.L3) should be used.  The single bug I found is a port bug, and it wants to sometimes use gs() and sometimes not use gs().  You can't do that.  For the totality of what people want to work, you have to either use gs() everywhere, or nowhere; it is the mixture that is the problem.  And then, the only thing that bug prevents is one test case from running.  That test case could be skipped on avr, if it doesn't work reliably, as most other port maintainers would rather be consistent, and if they are, they can't hit this avr port bug.

Also, the code produced isn't run, so arguing what would happen when run is silly, as, that's not the case in any of the tests except one of them.  For the last one, there would appear to be a compiler code-gen bug that needs fixing as mentioned above.

Re: [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?

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

[Mike Stump]

On Nov 4, 2016, at 2:35 AM, Georg-Johann Lay <avr@gjlay.de> wrote:
> 
>> .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.

> Using gs, the expression would be gs(.L3)-(gs(.L2)) as gs implements word addresses, not byte addresses.

As I said, I tried that and could not get it to work.

>> The compile only test cases work fine now, so nothing needs to be done for them.
> 
> ICE is not a nice approach

I never saw an ICE with current release binutils and top of tree gcc.  If you want me to see an ICE and comment on it, I would need the command line apparently.

> , IMO.  I'd prefer some more graceful error using code locations of the labels (which are not available).

My preference is code-gen.  :-)  Anyway, the change I posted creates nice graceful errors, if one wants.

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

[Senthil Kumar Selvaraj]

Georg-Johann Lay writes:

> 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...
>
> Bottom line is that label offsets are not supported by avr BE, and the 
> intention of the patch is to reduce FAIL noise in testsuite results because of 
> the missing support.
>
> If a dg-require is not appropriate, should I rather add dg-skip-if to every 
> test case?  I'd still prefer the dg-require solution because if the Binutils 
> will ever support label differences, then the testsuite will automatically 
> catch up with that.
>
>> thus is valid for all 16-bit one contexts.  The fact the order between the
>> stub and the actual code is different is irrelevant, it is a private
>
> Only if the code is not executed; there are several test cases that compute 
> relative placements of labels like:
>
> x(){if(&&e-&&b<0)x();b:goto*&&b;e:;}
>
> If a test ever relies on the fact that &&e - &&b tells anything about the 
> ordering of the labels, the code is about to crash.
>
>> implementation detail of the port, the point is the semantics are fixed and
>> constant and useful.  In deed that there is even a stub is a private
>> implementation detail of the port.  I think the `extra' helpful warning from
>> avr_print_operand_address is wrong and should be removed.  Think of the
>
> The following code simple makes absolutely no sense in the presence of stubs:
> int
> test (int foo)
> {
>    static void *dummy[] = { &&a, &&b };
>    goto *((char *) &&b - 2 * (foo < 0));
> a:
> b:
>    return 0;
> }
>
> It's only a compile test (the original PR66123 is about ICE), but the compiler 
> cannot know that it's just a crazy test case that won't be executed...
>
> When you are offsetting from a stub you might end up *anywhere*, even in some 
> data range.
>
>> label as gs(LAB), not LAB, burn LAB from your mind.  Once you do that, you
>> see you can't talk about the order LAB1 > LAB2, the concept doesn't make any
>> sense.  The _only_ think you can talk about is gs(LAB1) > gs(LAB2).  And
>> because of that, it is always consistent and works just fine.
>>
>> Once that misguided complains from gcc and bisutils are fixed, are their any
>> failing cases?
>
> State of matters is that Binutils support is missing, and if I understand you 
> correctly, dg-require is not appropriate to factor out availability of such 
> features?

I'll take a stab at adding the missing binutils support for avr label diffs.

Regards
Senthil

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

[Georg-Johann Lay]

On 04.11.2016 06:18, Senthil Kumar Selvaraj wrote:
>
> Georg-Johann Lay writes:
>> State of matters is that Binutils support is missing, and if I understand you
>> correctly, dg-require is not appropriate to factor out availability of such
>> features?
>
> I'll take a stab at adding the missing binutils support for avr label diffs.

Thanks for taking care of this.  I had a look into it but got stuck with a test 
case derived from gcc.c-torture/execute/pr70460.c

int c;

__attribute__((noinline, noclone)) void
call (void)
{
   __asm ("nop");
}

__attribute__((noinline, noclone)) void
foo (int x)
{
   static int b[] = { &&lab1 - &&lab0, &&lab2 - &&lab0 };
   void *a = &&lab0 + b[x];
   goto *a;
lab1:
   c += 2;
   call();
lab2:
   c++;
lab0:
   ;
}

int
main ()
{
   foo (0);
   if (c != 3)
     __builtin_abort ();
   foo (1);
   if (c != 4)
     __builtin_abort ();
   return 0;
}


The problem is when relaxation is turned on and the CALL is shortened to  RCALL 
but respective literals are not fixed up.

Johann

> Regards
> Senthil

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

[Senthil Kumar Selvaraj]

Georg-Johann Lay writes:

> On 04.11.2016 06:18, Senthil Kumar Selvaraj wrote:
>>
>> Georg-Johann Lay writes:
>>> State of matters is that Binutils support is missing, and if I understand you
>>> correctly, dg-require is not appropriate to factor out availability of such
>>> features?
>>
>> I'll take a stab at adding the missing binutils support for avr label diffs.
>
> Thanks for taking care of this.  I had a look into it but got stuck with a test 
> case derived from gcc.c-torture/execute/pr70460.c
>
> int c;
>
> __attribute__((noinline, noclone)) void
> call (void)
> {
>    __asm ("nop");
> }
>
> __attribute__((noinline, noclone)) void
> foo (int x)
> {
>    static int b[] = { &&lab1 - &&lab0, &&lab2 - &&lab0 };
>    void *a = &&lab0 + b[x];
>    goto *a;
> lab1:
>    c += 2;
>    call();
> lab2:
>    c++;
> lab0:
>    ;
> }
>
> int
> main ()
> {
>    foo (0);
>    if (c != 3)
>      __builtin_abort ();
>    foo (1);
>    if (c != 4)
>      __builtin_abort ();
>    return 0;
> }
>
>
> The problem is when relaxation is turned on and the CALL is shortened to  RCALL 
> but respective literals are not fixed up.

That linker issue is fixed with https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=4cb771f214ed6a2102e37bce255c6be5d0642f3a

Regards
Senthil