On Sat, Jan 19, 2013 at 1:15 AM, Vladimir Makarov wrote: > On 13-01-17 6:45 PM, Steven Bosscher wrote: >> >> Hello Vlad, >> >> Attached is my attempt to fix PR55934, an error recovery issue in LRA >> with incorrect constraints in an asm. >> >> I'm not 100% sure this is all correct (especially the LRA insn data >> invalidating in lra-assigns.c) but it appears to fix the PR without >> introducing test suite failures. > > The code is correct but call lra_invalidate_insn_data is not necessary as > the same thing will be done in lra_update_insn_recog_data (if what > lra_invalidate_insn_data does is not done yet) .
That is what I expected, too. My first attempts didn't have the lra_invalidate_insn_data call. But I think lra_update_insn_recog_data calls lra_invalidate_insn_data for asms. lra_invalidate_insn_data is called if there is existing recog data but the insn code has changed: if ((data = lra_insn_recog_data[uid]) != NULL && data->icode != INSN_CODE (insn)) { invalidate_insn_data_regno_info (data, insn, get_insn_freq (insn)); invalidate_insn_recog_data (uid); data = NULL; } For an asm, INSN_CODE==-1, so "data->icode != INSN_CODE (insn)" is always false, and lra_invalidate_insn_data is never called. The result is an ICE: pr55512-3.c:15:1: internal compiler error: in lra_update_insn_recog_data, at lra.c:1263 lra.c:1263 lra_assert (nop == data->insn_static_data->n_operands); > So adding the additional > call is harmless as the result will be the same. Given my explanation above, do you think we should make lra_update_insn_recog_data handle asms as a special case? E.g.: Index: lra.c =================================================================== --- lra.c (revision 195104) +++ lra.c (working copy) @@ -1239,7 +1239,8 @@ lra_update_insn_recog_data (rtx insn) check_and_expand_insn_recog_data (uid); if ((data = lra_insn_recog_data[uid]) != NULL - && data->icode != INSN_CODE (insn)) + && (data->icode != INSN_CODE (insn) + || asm_noperands (PATTERN (insn)) >= 0)) { invalidate_insn_data_regno_info (data, insn, get_insn_freq (insn)); invalidate_insn_recog_data (uid); Or just keep the lra_invalidate_insn_data call? Ciao! Steven