"H.J. Lu" <hjl.to...@gmail.com> writes: > On Thu, Aug 6, 2015 at 11:19 AM, Richard Sandiford > <rdsandif...@googlemail.com> wrote: >> "H.J. Lu" <hongjiu...@intel.com> writes: >>> Since ira_implicitly_set_insn_hard_regs may be called outside of >>> ira-lives.c, it can't use the local variable, preferred_alternatives. >>> This patch adds an alternative_mask argument to >>> ira_implicitly_set_insn_hard_regs. >>> >>> OK for master and 5 branch if there are no regressions on Linux/x86-64? >> >> Thanks for working on this. The patch looks good to me FWIW. >> >> I think this version is safer than the second one you posted. With that >> version we could end up with the same sort of bug, e.g. because a function >> passes false to extract_insn without realising that one of the functions >> that it calls later needs the preferred alternatives to be set. >> Or maybe (as with my patch) a function starts to use preferred_alternatives >> and one of its callers gets missed. >> >> Sorry for the breakage. >> > > I think local variable preferred_alternatives in ira-lives.c is problematic > by itself. It is easy to become stale.recog_data_d is the natural place > for it.
Well, the idea was that preferred_alternatives was just for local (static) subroutines of process_bb_node_lives. When doing the original conversion I'd missed that ira_implicitly_set_insn_hard_regs isn't in that group and is instead a public routine. Within the process_bb_node_lives subgroup the use of recog_data is very tightly controlled (and only set in process_bb_nodes). I don't think there's a significant risk of preferred_alternatives becoming out of sync with recog_data within that group. > But as you pointed out, it has its own problem. I don't know if we > should always call get_preferred_alternatives to initialize it even if > it isn't used. I think we should try to avoid that. It's very compile-time-sensitive code. Thanks, Richard
