On Tue, Dec 23, 2014 at 6:59 PM, Jeff Law <l...@redhat.com> wrote: > On 12/23/14 19:47, H.J. Lu wrote: >> >> On Tue, Dec 23, 2014 at 6:16 PM, John David Anglin <dave.ang...@bell.net> >> wrote: >>> >>> On 23-Dec-14, at 7:28 PM, H.J. Lu wrote: >>> >>>> On Tue, Dec 23, 2014 at 3:55 PM, John David Anglin >>>> <dave.ang...@bell.net> >>>> wrote: >>>>> >>>>> >>>>> On 23-Dec-14, at 5:37 PM, H.J. Lu wrote: >>>>> >>>>>> In this case, arguments are passed in registers. Isn't the >>>>>> optimization >>>>>> disabled for ia32, which passes arguments on stack, even before your >>>>>> change? >>>>> >>>>> >>>>> >>>>> >>>>> It's not disabled in dse.c. Possibly, this occurs for some cases for >>>>> ia32 >>>>> in ix86_function_ok_for_sibcall. >>>>> >>>>> The problem in dse is in how to distinguish stores used for arguments >>>>> from >>>>> those used for general >>>>> manipulation of data. It seems the argument data for the call to bar >>>>> in >>>>> the >>>>> testcase are copied through >>>>> frame memory on x86_64. >>>>> >>>>> We have two situations: >>>>> >>>>> /* This field is only used for the processing of const functions. >>>>> These functions cannot read memory, but they can read the stack >>>>> because that is where they may get their parms. We need to be >>>>> this conservative because, like the store motion pass, we don't >>>>> consider CALL_INSN_FUNCTION_USAGE when processing call insns. >>>>> Moreover, we need to distinguish two cases: >>>>> 1. Before reload (register elimination), the stores related to >>>>> outgoing arguments are stack pointer based and thus deemed >>>>> of non-constant base in this pass. This requires special >>>>> handling but also means that the frame pointer based stores >>>>> need not be killed upon encountering a const function call. >>>>> 2. After reload, the stores related to outgoing arguments can be >>>>> either stack pointer or hard frame pointer based. This means >>>>> that we have no other choice than also killing all the frame >>>>> pointer based stores upon encountering a const function call. >>>>> This field is set after reload for const function calls. Having >>>>> this set is less severe than a wild read, it just means that all >>>>> the frame related stores are killed rather than all the stores. >>>>> */ >>>>> bool frame_read; >>>>> >>>>> Case 1 is incorrect for sibling calls as the call arguments are frame >>>>> or >>>>> argument pointer based >>>>> when they are not passed in registers. >>>>> >>>>> When we don't have a const function, dse assumes: >>>>> >>>>> /* Every other call, including pure functions, may read any >>>>> memory >>>>> that is not relative to the frame. */ >>>>> add_non_frame_wild_read (bb_info); >>>>> >>>>> Again, this is incorrect for sibling calls (i.e., dse in general >>>>> assumes >>>>> that call arguments are register >>>>> or stack pointer based before reload). >>>>> >>>> >>>> This optimization is done on purpose to fix: >>>> >>>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=44194 >>>> >>>> Unless it is also incorrect for x86, why not only disable it for >>>> your target without penalizing x86 through a target hook. >>> >>> >>> >>> >>> I tend to think the old code was wrong for x86 but I recognize the lost >>> optimization. >>> >>> This isn't an optimization issue on hppa. It's a wrong code bug. >> >> >> I run gcc.dg/pr55023.c on x86-64 and x86 without your fix. It works >> fine. Can you find a testcase, failing on x86-64 and x86, which is >> fixed by your change? If not, this bug doesn't affect x86 and you >> should find another way to fix your target problem without introducing >> a regression on x86. > > Just to clarify, this is not a target problem, this is a problem that DSE is > not designed to handle certain situations WRT argument stores that occur on > targets with certain properties.
Why not limit the change to targets with those certain properties? -- H.J.
