| Message ID | 53E33AE9.4050902@arm.com |
|---|---|
| State | New |
| Headers | show |
On Aug 7, 2014, at 1:38 AM, Kyrill Tkachov <kyrylo.tkachov@arm.com> wrote: > > Thanks for the detailed explanation, the linker errors I was seeing were about relocations being truncated. Ah, those are bugs in your port! You should be able to generate large code and then relax it into short small code. Large code, by definition, will never run into relocs being truncated. :-) > I've extended your patch to catch those as well. With this the tests I was seeing FAIL now are marked UNSUPPORTED. > How is this? No. :-( Those are traditionally bugs in gcc that users want gcc to fix. Paper overing those bugs is the wrong path forward. So, a couple of ideas come to mind. The best, add relation and generate large by default. Next solution, is to have a linker script that limits memory to the size that the large reloc supports. If it is 18 bits, then limit memory to 18 bits. Doesn’t impact normal users as they only have 18 bits or less on their system. Next up, add a -mcmodel=large and make it the default and have people that want small code use -mcmodel=small. Another solution is to add a non-default -mc-model=large, and generate large code with that option, and then fix the specific test cases in the gcc test suite that fail to use that option on your target. This is a small maintenance nightmare, but… So, which one do you like? The model option (I just got done doing one for mine). I was building gcc for my target, which only the simulator can run due to memory sizes and hit the relocs don’t fit. I fixed it by 2 lines of work, one in branch and one in call, that removed the displacement forms under large model. Generates gross code, but I only need it for testing. For production, we default to and use small. The reality is that while the other instruction might theoretically hit the reloc limits, in practice they don’t.
On 08/08/14 18:53, Mike Stump wrote: > On Aug 7, 2014, at 1:38 AM, Kyrill Tkachov <kyrylo.tkachov@arm.com> wrote: >> >> Thanks for the detailed explanation, the linker errors I was seeing were about relocations being truncated. > > Ah, those are bugs in your port! You should be able to generate large code and then relax it into short small code. Large code, by definition, will never run into relocs being truncated. :-) > >> I've extended your patch to catch those as well. With this the tests I was seeing FAIL now are marked UNSUPPORTED. > >> How is this? > > No. :-( > > Those are traditionally bugs in gcc that users want gcc to fix. Paper overing those bugs is the wrong path forward. > Not quite, read the subject line again. This particular case the user (test) has asserted that the image will fit in a certain amount of memory; but in fact that turns out to be false. This can only be detected at link time when the relocations overflow - it's by definition impossible to detect during the compilation phase. I'm not sure what the correct change to the testsuite is here. Any test failures like this are likely to be somewhat target specific. In the worst case the optimization level may affect what can be made to fit. Perhaps the best solution would be something like marking the test as "large" in some way and for "large" tests the linker would handle "relocation truncated to fit" errors from the linker through some target hook that had a better understanding of whether size related options were being used and could decide between error and unsupported. R. > So, a couple of ideas come to mind. The best, add relation and generate large by default. Next solution, is to have a linker script that limits memory to the size that the large reloc supports. If it is 18 bits, then limit memory to 18 bits. Doesn’t impact normal users as they only have 18 bits or less on their system. Next up, add a -mcmodel=large and make it the default and have people that want small code use -mcmodel=small. Another solution is to add a non-default -mc-model=large, and generate large code with that option, and then fix the specific test cases in the gcc test suite that fail to use that option on your target. This is a small maintenance nightmare, but… > > So, which one do you like? > > The model option (I just got done doing one for mine). I was building gcc for my target, which only the simulator can run due to memory sizes and hit the relocs don’t fit. I fixed it by 2 lines of work, one in branch and one in call, that removed the displacement forms under large model. Generates gross code, but I only need it for testing. For production, we default to and use small. The reality is that while the other instruction might theoretically hit the reloc limits, in practice they don’t. >
On Aug 11, 2014, at 2:06 AM, Richard Earnshaw <rearnsha@arm.com> wrote: > Not quite, read the subject line again. Doh. I did miss that entirely. The solutions I gave were for other cases than the case at hand. > I'm not sure what the correct change to the testsuite is here. The below is close, let me refine it a little. > Perhaps the best solution would be something like marking the test as > "large" in some way and for "large" tests the linker would handle > "relocation truncated to fit" errors from the linker through some target > hook that had a better understanding of whether size related options > were being used and could decide between error and unsupported. How about a target tiny in supports.exp and any target that is tiny, we handle overflows in relocs as always unsupported. Works for all tiny targets, and uniformly works for all languages and all test cases of all time. Doesn’t depend upon guessing a size (how many bytes is tiny, is it code or data, and exactly how many bytes were generated on the target for the test case) nor guessing which test case are large. If you test the entire test suite with the tiny flag or if that flag is the default, then supports will say that the target is tiny. If you don’t give that flag and it isn’t the default, that same target is large. A person that only has tiny, can just say I’m tiny, and be forever done with it. An advanced ports with relaxation can then remove the I’m tiny, and then test relaxation. I think that offers little code to do this (5-10 lines), handles most situations nicely, retains as much testing as possible generally speaking. If one wants to handle mcmodel options on test cases seamlessly, one can use check-flags I think as well, see check_effective_target_arm_fp16_ok_nocache for example. Something like: proc ${tool}_check_unsupported_p { output } { if [regexp "(^|\n)\[^\n\]*: region \[^\n\]* is full" $output] { return "memory full” + if { [regexp "(^|\n)\[^\n\]*: relocation truncated to fit" $output] && [check_effective_target_tiny] } { return "memory full” } proc check_effective_target_tiny { } { if { [istarget blabla-*-*] return 1 } return 0 } if the choice is static for the target. Slightly more complex is check-flags is used. I’ll leave that as an exercise for the reader. :-)
diff --git a/gcc/testsuite/lib/gcc-defs.exp b/gcc/testsuite/lib/gcc-defs.exp index 69a5971..8ea1f55 100644 --- a/gcc/testsuite/lib/gcc-defs.exp +++ b/gcc/testsuite/lib/gcc-defs.exp @@ -154,7 +154,8 @@ proc ${tool}_exit { } { # proc ${tool}_check_unsupported_p { output } { - if [regexp "(^|\n)\[^\n\]*: region \[^\n\]* is full" $output] { + if { [regexp "(^|\n)\[^\n\]*: region \[^\n\]* (is full|overflowed by )" $output] \ + || [regexp "(^|\n)\[^\n\]*: relocation truncated to fit" $output] } { return "memory full" } if { [istarget spu-*-*] && \ diff --git a/gcc/testsuite/lib/gcc-dg.exp b/gcc/testsuite/lib/gcc-dg.exp index 3390caa..d8f921a 100644 --- a/gcc/testsuite/lib/gcc-dg.exp +++ b/gcc/testsuite/lib/gcc-dg.exp @@ -225,10 +225,13 @@ proc gcc-dg-prune { system text } { } } - # If we see "region xxx is full" then the testcase is too big for ram. - # This is tricky to deal with in a large testsuite like c-torture so - # deal with it here. Just mark the testcase as unsupported. - if [regexp "(^|\n)\[^\n\]*: region \[^\n\]* is full" $text] { + # If we see "region xxx is full" or "region xxx overflowed by " + # or "relocation truncated to fit" + # then the testcase is too big for ram. This is tricky to deal + # with in a large testsuite like c-torture so deal with it here. + # Just mark the testcase as unsupported. + if { [regexp "(^|\n)\[^\n\]*: region \[^\n\]* (is full|overflowed by )" $text] \ + || [regexp "(^|\n)\[^\n\]*: relocation truncated to fit" $text] } { # The format here is important. See dg.exp. return "::unsupported::memory full" } diff --git a/gcc/testsuite/lib/objc.exp b/gcc/testsuite/lib/objc.exp index 5ecefa9..45d9de1 100644 --- a/gcc/testsuite/lib/objc.exp +++ b/gcc/testsuite/lib/objc.exp @@ -354,7 +354,8 @@ if { [info procs prune_warnings] == "" } then { # gld so we can tell what the error text will look like. proc ${tool}_check_unsupported_p { output } { - if [regexp "(^|\n)\[^\n\]*: region \[^\n\]* is full" $output] { + if { [regexp "(^|\n)\[^\n\]*: region \[^\n\]* (is full|overflowed by )" $output] \ + || [regexp "(^|\n)\[^\n\]*: relocation truncated to fit" $output] } { return "memory full" } return ""