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?
Thanks,
Christophe
> Roman
>
>
>
>
