| Message ID | 968cb4a4-ff85-ebaa-6cd2-98ebe9851f6c@codesourcery.com |
|---|---|
| State | New |
| Headers | show |
| Series | [Patch?,RFC,RTL] clobber handling & buildin expansion - missing insn_invalid_p call [PR100418] | expand |
On 5/5/2021 7:50 AM, Tobias Burnus wrote: > Hi Eric, hi all, > > currently, gcn (amdgcn-amdhsa) bootstrapping fails as Alexandre's > patch to __builtin_memset (applied yesterday) now does more expansions. > > The problem is [→ PR100418] > (set(reg:DI)(plus:DI(reg:DI)(const_int))) [= "adddi3"] > This fails with gcn as gcn has two clobbers for "adddi3" - and when > expand_insn > is called, INSN_CODE == -1 via: > icode = recog_memoized (insn); > alias > INSN_CODE (insn) = recog (PATTERN (insn), insn, 0); > As the "int *pnum_clobber" argument is NULL (well, '0'), the > clobbers are not available - which causes the pattern fail. > > I think that's a general issue with the RTX code generated by > builtins.c – except that most targets either do not > have clobbers for the used operators — or the code is by > chance fixed: > > For instance, I see that several "if" blocks being processed in > recog.c's insn_invalid_p via 'cleanup_cfg (CLEANUP_NO_INSN_DEL)'; > the innermost parts of the call chain are: > apply_change_group → verify_changes → insn_invalid_p > > * * * > > The attached patch seems to solve the GCN issue. Does it look OK? > > Or does the insn_invalid_p call come too late? > If so, any suggestion where it would fit best? > > Tobias, > who is more a FE and early-ME person. > > ----------------- > Mentor Graphics (Deutschland) GmbH, Arnulfstrasse 201, 80634 München > Registergericht München HRB 106955, Geschäftsführer: Thomas Heurung, > Frank Thürauf > > recog.diff > > extract_insn: Call insn_invalid_p is insn cannot be found. > > gcc/ChangeLog: > > * recog.c (extract_insn): Call insn_invalid_p if > recog_memoized did not find the insn. Was this issue on GCN fixed by Andrew/Jakub's change to use force_operand rather than emit_move_insn? Jeff
On 30/05/2021 19:51, Jeff Law wrote: > > > On 5/5/2021 7:50 AM, Tobias Burnus wrote: >> Hi Eric, hi all, >> >> currently, gcn (amdgcn-amdhsa) bootstrapping fails as Alexandre's >> patch to __builtin_memset (applied yesterday) now does more expansions. >> >> The problem is [→ PR100418] >> (set(reg:DI)(plus:DI(reg:DI)(const_int))) [= "adddi3"] >> This fails with gcn as gcn has two clobbers for "adddi3" - and when >> expand_insn >> is called, INSN_CODE == -1 via: >> icode = recog_memoized (insn); >> alias >> INSN_CODE (insn) = recog (PATTERN (insn), insn, 0); >> As the "int *pnum_clobber" argument is NULL (well, '0'), the >> clobbers are not available - which causes the pattern fail. [...] > Was this issue on GCN fixed by Andrew/Jakub's change to use > force_operand rather than emit_move_insn? Yes, that's what that patch fixed. Tobias's patch would also fix the issue, and any future issues with a similar problem. Andrew
extract_insn: Call insn_invalid_p is insn cannot be found. gcc/ChangeLog: * recog.c (extract_insn): Call insn_invalid_p if recog_memoized did not find the insn. diff --git a/gcc/recog.c b/gcc/recog.c index eb617f11163..4ddc5d185af 100644 --- a/gcc/recog.c +++ b/gcc/recog.c @@ -2766,6 +2766,8 @@ extract_insn (rtx_insn *insn) and get the constraints. */ icode = recog_memoized (insn); + if (icode < 0 && !insn_invalid_p (insn, false)) + icode = INSN_CODE (insn); if (icode < 0) fatal_insn_not_found (insn);