[Bug rtl-optimization/112560] [14 Regression] ICE in try_combine on pr112494.c
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=112560
--- Comment #13 from Uroš Bizjak ---
(In reply to Segher Boessenkool from comment #12)
> You cannot use a :CC value as argument of an unspec, as explained before.
>
> The result of a comparison is expressed as a comparison, in RTL. This patch
> allows malformed RTL in more places than before, not progress at all.
During our discussion we determined that this form with UNSPEC is actually a
copy operation, so it is not an use [1] of CC register, because "use" is in the
form of cc-compared-to-0.
[1] https://gcc.gnu.org/pipermail/gcc-patches/2024-March/647426.html
Now, let's see the part my patch fixes. The original change that introduced the
functionality (See Comment #1) updates the "use" of the CC register. It assumes
exclusively cc-compared-to-0 use, but there are several patterns in various
target .md files that use naked CC register. The "???" comment suggests that
the transformation tripped on this and thus the "verify the zero_rtx" was
bolted on. The zero_rtx is inherent part of the regular CC reg "use", so this
addition _mostly_ weeded out invalid access with naked CC reg.
If the CC reg is used as the source of copy operation ("move"), with or without
UNSPEC, then the unpatched compiler will blindly use:
SUBST (XEXP (*cc_use_loc, 0), newpat_dest);
which *assumes* the certain form of the changed expression. Failed assumption
will lead to memory corruption, and this is what my patch prevents.
Patched compiler will find the sole use of the naked CC reg (due to
find_single_use) in the RTX, and update its mode at the right place. If the new
mode is not recognized by the insn pattern, then the combination is rejected.
In any case, we are trading silent memory corruption with failed combine
attempt. In my rule book, this is "progress" with bold, capital letters.
[Bug rtl-optimization/112560] [14 Regression] ICE in try_combine on pr112494.c
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=112560 --- Comment #12 from Segher Boessenkool --- You cannot use a :CC value as argument of an unspec, as explained before. The result of a comparison is expressed as a comparison, in RTL. This patch allows malformed RTL in more places than before, not progress at all.
[Bug rtl-optimization/112560] [14 Regression] ICE in try_combine on pr112494.c
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=112560
--- Comment #11 from Uroš Bizjak ---
(In reply to Segher Boessenkool from comment #10)
> It is still wrong. You're trying to sweep tour wrong assumptions under the
> rug,
> but they will only rear up elsewhere. Just fix the actual *target* problem!
I can't see what could be wrong with:
(define_insn "@pushfl2"
[(set (match_operand:W 0 "push_operand" "=<")
(unspec:W [(match_operand 1 "flags_reg_operand")]
UNSPEC_PUSHFL))]
"GET_MODE_CLASS (GET_MODE (operands[1])) == MODE_CC"
"pushf{}"
[(set_attr "type" "push")
(set_attr "mode" "")])
it is just a push of the flags reg to the stack. If the push can't be
described in this way, then it is the middle end at fault, we can't
just change modes at will.
[Bug rtl-optimization/112560] [14 Regression] ICE in try_combine on pr112494.c
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=112560 --- Comment #10 from Segher Boessenkool --- It is still wrong. You're trying to sweep tour wrong assumptions under the rug, but they will only rear up elsewhere. Just fix the actual *target* problem!
[Bug rtl-optimization/112560] [14 Regression] ICE in try_combine on pr112494.c
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=112560 --- Comment #9 from Uroš Bizjak --- (In reply to Richard Biener from comment #8) > Fixed I suppose. Yes - I plan backport the patch to at least gcc-13.
[Bug rtl-optimization/112560] [14 Regression] ICE in try_combine on pr112494.c
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=112560 Richard Biener changed: What|Removed |Added Status|ASSIGNED|RESOLVED Resolution|--- |FIXED --- Comment #8 from Richard Biener --- Fixed I suppose.
[Bug rtl-optimization/112560] [14 Regression] ICE in try_combine on pr112494.c
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=112560 --- Comment #7 from GCC Commits --- The master branch has been updated by Uros Bizjak : https://gcc.gnu.org/g:eaccdba315b86d374a4e72b9dd8fefb0fc3cc5ee commit r14-9847-geaccdba315b86d374a4e72b9dd8fefb0fc3cc5ee Author: Uros Bizjak Date: Mon Apr 8 20:54:30 2024 +0200 combine: Fix ICE in try_combine on pr112494.c [PR112560] The compiler, configured with --enable-checking=yes,rtl,extra ICEs with: internal compiler error: RTL check: expected elt 0 type 'e' or 'u', have 'E' (rtx unspec) in try_combine, at combine.cc:3237 This is 3236 /* Just replace the CC reg with a new mode. */ 3237 SUBST (XEXP (*cc_use_loc, 0), newpat_dest); 3238 undobuf.other_insn = cc_use_insn; in combine.cc, where *cc_use_loc is (unspec:DI [ (reg:CC 17 flags) ] UNSPEC_PUSHFL) combine assumes CC must be used inside of a comparison and uses XEXP (..., 0) without checking on the RTX type of the argument. Replace cc_use_loc with the entire new RTX only in case cc_use_loc satisfies COMPARISON_P predicate. Otherwise scan the entire cc_use_loc RTX for CC reg to be updated with a new mode. PR rtl-optimization/112560 gcc/ChangeLog: * combine.cc (try_combine): Replace cc_use_loc with the entire new RTX only in case cc_use_loc satisfies COMPARISON_P predicate. Otherwise scan the entire cc_use_loc RTX for CC reg to be updated with a new mode. * config/i386/i386.md (@pushf2): Allow all CC modes for operand 1.
[Bug rtl-optimization/112560] [14 Regression] ICE in try_combine on pr112494.c
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=112560 --- Comment #6 from Uroš Bizjak --- v3 patch at [1] [1] https://gcc.gnu.org/pipermail/gcc-patches/2024-March/647634.html
[Bug rtl-optimization/112560] [14 Regression] ICE in try_combine on pr112494.c
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=112560 Uroš Bizjak changed: What|Removed |Added Attachment #56705|0 |1 is obsolete|| --- Comment #5 from Uroš Bizjak --- Created attachment 57666 --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=57666=edit Proposed v2 patch New version of patch in testing. This version handles change of the mode of CC reg outside comparison. Now we scan the RTX and change the mode of the CC reg at the proper place. We are guaranteed by find_single_use that cc_use_loc can be non-NULL only when exactly one user follows the combined comparison. In case of unsupported cc_use_insn combine will be undone. To avoid combine failure, pushfl2 in i386.md is changed to accept all MODE_CC modes.
