Hi,
Christophe Lyon wrote 2020-03-27 19:53:
Hi,
On Fri, 20 Mar 2020 at 11:56, Roman Zhuykov via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
Hi all!
12.03.2020 6:17, Jeff Law wrote:
>> Current modulo-sched implementation is a bit faulty from -fcompile-debug
>> perspective.
>>
>> But right now I see that when I enable -fmodulo-sched by default, powerpc64le
>> bootstrap give comparison failure as of r10-7056.
>>
>> Comparing stages 2 and 3
>> Bootstrap comparison failure!
>> gcc/ggc-page.o differs
>>
>> That doesn't happen on released branches, so it is a kind of "regression"
>> (certainly, nobody runs bootstrap with -fmodulo-sched).
> Even though we don't have a BZ, I think a bootstrap failure, even one with
> modulo-scheduling enabled is severe enough that we should try to fix it.
>
> OK for the trunk.
>
> jeff
11.03.2020 11:14, Richard Biener wrote:
>
> Yes from a RM perspective, no comments about the technical details of
> the patch.
>
> Richard.
>
Haven't committed that patch yet but made some additional
investigation.
(0) Running tests with -fcompare-debug (without -fmodulo-sched) shows
a lot of different failures, I've created few PRs already and they
were addressed (Thanks to Jakub!).
There are three kinds of issues.
(0a) Technical ones, I've not reported them. Many checking techniques
are not ready for the compiler to run twice for one driver invocation.
E.g.
FAIL: c-c++-common/diagnostic-format-json-2.c -Wc++-compat (test for
excess errors)
fails because second cc1 run prints additional '[]' to the output.
All tests in gcc.dg/pch directory have a similar issue.
(0b) When only dumps differ, but not the code, e.g. PR94223 and
PR94167.
(0c) When it is a real "mistake", like PR94166 or PR94211. Or -w
difference like in PR94189.
Now, speaking about SMS, enabling it by default catches three (again!)
kinds of separate issues with debug instructions. And two first are
observable for many years, while the third one is a bit more tricky
and I caught it only in 2019. My previous patch targeted (2) and (3)
together, and now I have to split it.
(1) The first issue is that reordering doesn't touch debug
instructions at all. When we have found a proper schedule
permute_partial_schedule function is called. It constructs new order
in reverse, first we move the instruction that should be prev_insn
(jump) to its place, than the previous one, etc.
It doesn't consider moving debug instructions, so all of them are
crowded before any non-debug instruction. This certainly doesn't help
much for debug information and on ppc64le causes
FAIL: gcc.dg/guality/loop-1.c -O2 -DPREVENT_OPTIMIZATION line 20 i
== 1
and few other failures. I never actually tried to fix those, maybe
the best solution here is to drop all debug inside the loop when SMS
succeeded. But in this case we will also lose something - at least
for now we have correct debug info when the loop was finished.
(2) The second issue is a "simple" -fcompare-debug failure, which
actually can not lead to different code in .text (and can not cause
bootstrap failure). As I mentioned in previous mail it can be
observed in pr42631.c example for ~10 years. My previous patch and
small attached patch2.txt (can be applied only after patch3.txt)
solves this. Note instructions "stick" to nearest following
instruction and they are moved all together. But when one compares
-g0 and -g, the note can instead stick to a debug instruction, and
according to (1) it will eventually "move up" together with that debug
instruction instead of "sinking" with the following non-debug
instruction. An obvious solution is to extend the "sticking" approach
to debug instructions themselves. I've used that solution ("previous"
patch) locally for ~3 years. This change also affects the approach
described in (1) but unfortunately doesn't fix any testcase and make
something even worse:
FAIL: gcc.dg/guality/pr90716.c -O1 -DPREVENT_OPTIMIZATION line 23
j + 1 == 9
So, IMHO we should not apply this in stage4.
(3) And the last one we have to fix now if we bother about
-fmodulo-sched ppc64le bootstrap. Failing examples are "gcc
-fcompare-debug -O2 -fmodulo-sched --param sms-min-sc=1
gcc/testsuite/gcc.dg/torture/pr87197.c" on ppc64le and "gcc
-fcompare-debug -O2 gcc.c-torture/execute/pr70127.c" on aarch64.
Contrary to scheduler itself, when building DDG graph we consider all
debug instructions and their dependencies. This may sometimes lead to
a different result in sms_order_nodes function, as it depends on SCCs
in DDG. My previous ad-hoc solution was just to remove any
"debug->non-debug" edges in DDG. Now I reimplemented that by fully
removing debug instructions nodes. I've tested patch3.txt a lot last
week in different scenarios and will commit it in a week if no
objection.
I've noticed failures on arm and aarch64:
FAIL: gcc.dg/torture/pr87197-debug-sms.c -O3 -fomit-frame-pointer
-funroll-loops -fpeel-loops -ftracer -finline-functions (test for
excess errors)
Excess errors:
xgcc: error: /gcc/testsuite/gcc.dg/torture/pr87197-debug-sms.c:
'-fcompare-debug' failure
And on some arm targets:
FAIL: gcc.c-torture/execute/pr70127-debug-sms.c -O3
-fomit-frame-pointer -funroll-loops -fpeel-loops -ftracer
-finline-functions (test for excess errors)
Excess errors:
xgcc: error: /gcc/testsuite/gcc.c-torture/execute/pr70127-debug-sms.c:
'-fcompare-debug' failure
(--target arm-none-linux-gnueabihf --with-cpu cortex-a57 --with-fpu
crypto-neon-fp-armv8)
Are they still expected?
Thank you for reporting!
I have just pushed a testsuite adjustment into master (r10-7445), and
will give a brief explanation here few days later.
Roman
