[Bug rtl-optimization/99114] [WORD_REGISTER_OPERATIONS] wrong code for (u16_var & 3) == (u32)1
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99114 --- Comment #7 from pipcet at gmail dot com --- (In reply to Eric Botcazou from comment #4) > > I'll try, but please consider investigating this without one. It happens > > after a very lengthy compilation process (compiling a buggy gcc with a buggy > > cross-compiler, then compiling JIT code with that, generating a buggy .so, > > then running _that_, to see incorrect behavior rather than a clear crash), > > in C++ code, with an out-of-tree backend. > > Impressive indeed, but not without a precedent. Can you upload the RTL dump > files of the pass preceding .combine and that of .combine itself? Sure. I've tried taking the .i and compiling it with other backends, but they never seem to optimize away the inner (and:HI ...). > > On the other hand, it's been investigated, and it's a clear bug with a > > one-line fix. > > This needs to be double checked though, this code has been there for ages. I agree, of course. While I still believe that the problem is as I described, it's not as severe a bug as I initially thought: it essentially requires the architecture to have WORD_REGISTER_OPERATIONS but also memory compares, a very unusual combination for real hardware. (In reply to Richard Biener from comment #3) > If > > (gtu (subreg:SI (reg:HI 593)) (const_int 1)) > > is incorrect, why is it then recognized? And why should (subreg:SI (and:HI > ..)) > be OK? My understanding is it's valid but essentially meaningless RTL, so it should be okay to recognize. > The way I read WORD_REGISTER_OPERATIONS it's a bad design to make the IL do > something that could have better been explicitely represented. (if it is > a SImode op then please make it an SImode op!) Maybe a first step would be to add a "when in doubt, don't define it" remark to the documentation for WORD_REGISTER_OPERATIONS?
[Bug rtl-optimization/99114] [WORD_REGISTER_OPERATIONS] wrong code for (u16_var & 3) == (u32)1
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99114
--- Comment #6 from pipcet at gmail dot com ---
Created attachment 50204
--> https://gcc.gnu.org/bugzilla/attachment.cgi?id=50204=edit
RTL dump of combine pass
(Gzipped because of the file size limit).
The relevant section is around this code:
(note 4724 4779 4727 667 [bb 667] NOTE_INSN_BASIC_BLOCK)
(note 4727 4724 4728 667 NOTE_INSN_DELETED)
(note 4728 4727 4729 667 NOTE_INSN_DELETED)
(jump_insn 4729 4728 4730 667 (set (pc)
(if_then_else (gtu (subreg:SI (reg:HI 799 [ hrsi$word_no ]) 0)
(const_int 1 [0x1]))
(label_ref 4783)
(pc))) "../../../src/gcc/gcc/hard-reg-set.h":311:7 20 {*cbranchsi4}
(int_list:REG_BR_PROB 39298956 (nil))
-> 4783)
allowing combination of insns 4727 and 4728
original costs 4 + 4 = 8
replacement cost 4
deferring deletion of insn with uid = 4727.
modifying insn i3 4728: r2326:SI=r799:HI#0&0x3
deferring rescan insn with uid = 4728.
allowing combination of insns 4728 and 4729
original costs 4 + 4 = 8
replacement cost 4
deferring deletion of insn with uid = 4728.
modifying insn i3 4729: pc={(gtu(r799:HI#0,0x1))?L4783:pc}
REG_BR_PROB 39298956
deferring rescan insn with uid = 4729.
[Bug rtl-optimization/99114] [WORD_REGISTER_OPERATIONS] wrong code for (u16_var & 3) == (u32)1
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99114
pipcet at gmail dot com changed:
What|Removed |Added
CC||pipcet at gmail dot com
--- Comment #5 from pipcet at gmail dot com ---
Created attachment 50203
--> https://gcc.gnu.org/bugzilla/attachment.cgi?id=50203=edit
RTL dump of last pre-combine pass
(Gzipped because of the file size limit). The relevant section is around this
code:
(note 4724 4779 4727 667 [bb 667] NOTE_INSN_BASIC_BLOCK)
(insn 4727 4724 4728 667 (set (reg:SI 2326)
(ashift:SI (subreg:SI (reg:HI 799 [ hrsi$word_no ]) 0)
(const_int 16 [0x10]))) "../../../src/gcc/gcc/hard-reg-set.h":311:7
10 {*assignsi_binop}
(nil))
(insn 4728 4727 4729 667 (set (reg:SI 2326)
(lshiftrt:SI (reg:SI 2326)
(const_int 16 [0x10]))) "../../../src/gcc/gcc/hard-reg-set.h":311:7
10 {*assignsi_binop}
(nil))
(jump_insn 4729 4728 4730 667 (set (pc)
(if_then_else (gtu (reg:SI 2326)
(const_int 1 [0x1]))
(label_ref 4783)
(pc))) "../../../src/gcc/gcc/hard-reg-set.h":311:7 20 {*cbranchsi4}
(expr_list:REG_DEAD (reg:SI 2326)
(int_list:REG_BR_PROB 39298956 (nil)))
-> 4783)
[Bug rtl-optimization/99114] [WORD_REGISTER_OPERATIONS] wrong code for (u16_var & 3) == (u32)1
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99114 --- Comment #2 from pipcet at gmail dot com --- (In reply to Eric Botcazou from comment #1) > Please provide a reproducer as documented in https://gcc.gnu.org/bugs I'll try, but please consider investigating this without one. It happens after a very lengthy compilation process (compiling a buggy gcc with a buggy cross-compiler, then compiling JIT code with that, generating a buggy .so, then running _that_, to see incorrect behavior rather than a clear crash), in C++ code, with an out-of-tree backend. On the other hand, it's been investigated, and it's a clear bug with a one-line fix. > > The assumption here is that op0 will be an (and:HI) after the first > > statement (and we assume (subreg:SI (and:HI ... (const_int 3))) is > > defined because of WORD_REGISTER_OPERATIONS) but it's actually > > simplified to be just the (reg:HI 593), and (subreg:SI (reg:HI 593)) > > is not defined. > > Paradoxical registers are defined under specific circumstances though. Thanks, I understand that. This isn't one of them. > > I'm unsure whether this can cause wrong code for in-tree backends or > > backends which don't define WORD_REGISTER_OPERATIONS. > > Well, obviously not for the latter, see the comment just above the code. As I said, I'm unsure. The buggy line of code is executed on other targets, and the condition under which that happens is not !paradoxical_subreg_p. I think it's equivalent, but I don't think that's obvious...
[Bug rtl-optimization/99114] New: [WORD_REGISTER_OPERATIONS] wrong code for (u16_var & 3) == (u32)1
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99114 Bug ID: 99114 Summary: [WORD_REGISTER_OPERATIONS] wrong code for (u16_var & 3) == (u32)1 Product: gcc Version: 11.0 Status: UNCONFIRMED Severity: normal Priority: P3 Component: rtl-optimization Assignee: unassigned at gcc dot gnu.org Reporter: pipcet at gmail dot com Target Milestone: --- I was seeing a miscompilation issue with my wasm32 backend (which was somewhat difficult to track down because it was actually the cross-compiled native compiler miscompiling JIT code). The symptom is that combine.c replaces the correct RTL (gtu (and:SI (subreg:SI (reg:HI 593)) (const_int 3)) (const_int 1)) with the incorrect RTL (gtu (subreg:SI (reg:HI 593)) (const_int 1)) when reg:HI 593 is known to be <= 3. The culprit is this code, in combine.c: op0 = simplify_gen_binary (AND, tmode, SUBREG_REG (XEXP (op0, 0)), gen_int_mode (c1, tmode)); op0 = gen_lowpart (mode, op0); The assumption here is that op0 will be an (and:HI) after the first statement (and we assume (subreg:SI (and:HI ... (const_int 3))) is defined because of WORD_REGISTER_OPERATIONS) but it's actually simplified to be just the (reg:HI 593), and (subreg:SI (reg:HI 593)) is not defined. I'm unsure whether this can cause wrong code for in-tree backends or backends which don't define WORD_REGISTER_OPERATIONS.
[Bug target/92729] [avr] Convert the backend to MODE_CC so it can be kept in future releases
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=92729 --- Comment #18 from pipcet at gmail dot com --- Sorry for only getting back to this now. I release all code on this branch into the public domain, if it helps at all. I'm happy to add whatever legal attribution is needed for that. I'm willing to help where I can, but please understand I've had to abandon this due to an unfortunate combination of family and health issues. I'm sorry about that.
