| Message ID | 4DA6CB8E.1040707@gjlay.de |
|---|---|
| State | New |
| Headers | show |
2011/4/14 Georg-Johann Lay <avr@gjlay.de>: > The "rotl<mode>3" expanders (mode \in {HI,SI,DI}) violates synopsis by > using 4 operands instead of 3. This runs in ICE in top of > optabs.c:maybe_gen_insn. > > The right way to do this is to use match_dup, not match_operand. So > the fix is obvious. > > Regenerated avr-gcc and run against avr testsuite: > > gcc.target/avr/torture/pr41885.c generates these patterns > > Johann > > 2011-04-14 Georg-Johann Lay <avr@gjlay.de> > > * config/avr/avr.md ("rotlhi3", "rotlsi3", "rotldi3"): Fix > expanders operand 3 from match_operand to match_dup. May be better to use (clobber (match_scratch ...)) here and in `*rotw<mode>' and `*rotb<mode>'. Denis.
Denis Chertykov schrieb: > 2011/4/14 Georg-Johann Lay <avr@gjlay.de>: >> The "rotl<mode>3" expanders (mode \in {HI,SI,DI}) violates synopsis by >> using 4 operands instead of 3. This runs in ICE in top of >> optabs.c:maybe_gen_insn. >> >> The right way to do this is to use match_dup, not match_operand. So >> the fix is obvious. >> >> Regenerated avr-gcc and run against avr testsuite: >> >> gcc.target/avr/torture/pr41885.c generates these patterns >> >> Johann >> >> 2011-04-14 Georg-Johann Lay <avr@gjlay.de> >> >> * config/avr/avr.md ("rotlhi3", "rotlsi3", "rotldi3"): Fix >> expanders operand 3 from match_operand to match_dup. > > May be better to use (clobber (match_scratch ...)) here and in > `*rotw<mode>' and `*rotb<mode>'. These are splitters that might split before reload, producing strange, non-matching insns like (set (scratch:QI) ... or crashing already in avr_rotate_bytes becase the split condition reads "&& (reload_completed || <MODE>mode == DImode)" By the way: this and other patterns implement DImode insns, but there is *no* movdi in avr backend. That is strongly discouraged, i.e. implementing insns for FOOmode if there in no movfoo insn that can do reloading for that mode. It's tempting to omit movdi (because movdi would be the most complicated DI insn) but it's unhealthy. IMO the DI stuff should be removed / deactivated as long as there is no proper support of DImode. > > Denis. >
2011/4/14 Georg-Johann Lay <avr@gjlay.de>: > Denis Chertykov schrieb: >> 2011/4/14 Georg-Johann Lay <avr@gjlay.de>: >>> The "rotl<mode>3" expanders (mode \in {HI,SI,DI}) violates synopsis by >>> using 4 operands instead of 3. This runs in ICE in top of >>> optabs.c:maybe_gen_insn. >>> >>> The right way to do this is to use match_dup, not match_operand. So >>> the fix is obvious. >>> >>> Regenerated avr-gcc and run against avr testsuite: >>> >>> gcc.target/avr/torture/pr41885.c generates these patterns >>> >>> Johann >>> >>> 2011-04-14 Georg-Johann Lay <avr@gjlay.de> >>> >>> * config/avr/avr.md ("rotlhi3", "rotlsi3", "rotldi3"): Fix >>> expanders operand 3 from match_operand to match_dup. >> >> May be better to use (clobber (match_scratch ...)) here and in >> `*rotw<mode>' and `*rotb<mode>'. > > These are splitters that might split before reload, producing strange, > non-matching insns like > (set (scratch:QI) ... > or crashing already in avr_rotate_bytes becase the split condition reads > "&& (reload_completed || <MODE>mode == DImode)" Generally I'm agree with you, change match_operand to match_dup is easy. But IMHO the right way is: - the splitters and avr_rotate_bytes are wrong and must be fixed too. - operands[3] is a scratch register and right way is to use match_scratch. I can approve the patch. Denis.
Index: config/avr/avr.md =================================================================== --- config/avr/avr.md (Revision 172416) +++ config/avr/avr.md (Arbeitskopie) @@ -1521,7 +1521,7 @@ (define_expand "rotl<mode>3" [(parallel [(set (match_operand:HIDI 0 "register_operand" "") (rotate:HIDI (match_operand:HIDI 1 "register_operand" "") (match_operand:VOID 2 "const_int_operand" ""))) - (clobber (match_operand 3 ""))])] + (clobber (match_dup 3))])] "" " {
The "rotl<mode>3" expanders (mode \in {HI,SI,DI}) violates synopsis by using 4 operands instead of 3. This runs in ICE in top of optabs.c:maybe_gen_insn. The right way to do this is to use match_dup, not match_operand. So the fix is obvious. Regenerated avr-gcc and run against avr testsuite: gcc.target/avr/torture/pr41885.c generates these patterns Johann 2011-04-14 Georg-Johann Lay <avr@gjlay.de> * config/avr/avr.md ("rotlhi3", "rotlsi3", "rotldi3"): Fix expanders operand 3 from match_operand to match_dup.