On Nov 3, 2016, at 8:25 AM, Georg-Johann Lay <a...@gjlay.de> wrote:
>
> On 28.10.2016 17:58, Mike Stump wrote:
>> On Oct 27, 2016, at 3:16 AM, Georg-Johann Lay <a...@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.
