On 04.11.2016 03:48, Mike Stump wrote:
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.
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?
