This is the mail archive of the gcc@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]

Re: the failure in store motion


Jan Hubicka <jh@suse.cz> writes:

>> (I also changed find_loads to look at set dests the same way
>> store_killed_in_insn does, so that it can handle parallels).
>> 
>> Even if you just tell it to remove available stores, we add testsuite
>> failures.
> Looking at the code, other place looks dubious:
> 
>   if (GET_CODE (PATTERN (insn)) == SET)
>     {
>       rtx pat = PATTERN (insn);
>       /* Check for memory stores to aliased objects.  */
>       if (GET_CODE (SET_DEST (pat)) == MEM && !expr_equiv_p (SET_DEST (pat), x))
> 	/* pretend its a load and check for aliasing.  */
> 	if (find_loads (SET_DEST (pat), x))
> 	  return 1;
>       return find_loads (SET_SRC (pat), x);
>     }
>   else
>     return find_loads (PATTERN (insn), x);
> 
> This code handles the real stores to an aliased objects by returning 1,
> but in case the instruction is not plain set, but it is an parallel containing
> set, it does not - it just calls find_loads, that hapilly ignores SET_DEST
> of the instructions.

Yes, I fixed this already, by moving similar code into find_loads.

> 
> One of oddities of i386 backend is the heavy use of PARALLELs, so this looks
> like probably bug in this area. Usually stores are just plain move
> instructions, that does not have the parallel, but even before GCSE there can
> be exceptions IMO.

Yup, it was a bug.
I'm doing somewhat better down.
I've got the redundant store elimination portion working, with no
regressions.

> 
> Whole gcse is somewhat PARALLEL unhappy. Once I will have more time, I will
> try to do pass over that and figure out what else needs to be changed.
> > 
>> I wonder if it has to do with trapping or something? (We kill trapping
>> stores on abnormal edges before performing the reverse LCM, but we
>> don't kill them before we remove available loads)
>> 
>> However, even if you comment out removing the redundant avail stores,
>> always set not transparent and killed if it's killed in a basic block,
>> etc, you *still* get wrong answers.
>> 
>> So store_killed_in_insn isn't quite right, even in the above version.
> Hope this helps,
> 
> Honza
> > 
>> --Dan
>> 
>> -- 
>> "When I was a baby, I kept a diary.  Recently, I was rereading
>> it.  It said, "Day 1 -- Still tired from the move.  Day 2 --
>> Everybody talks to me like I'm an idiot."
>> "-Steven Wright

-- 
"You know how it is when you're reading a book and falling
asleep, you're reading, reading...  And all of a sudden you
notice your eyes are closed?  I'm like that all the time.
"-Steven Wright


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]