Re: the failure in store motion

[Daniel Berlin <dan at cgsoftware dot com>]

Jan Hubicka <jh@suse.cz> writes:

>> Hmmm.
>> Let me give this a try.
>> As I said, it only fails miserably on x86, so it's likely something
>> very specific to that platform (like this type of thing).
>> Nope, doesn't help.
>> Even this, more conservative version doesn't work:
>> 
>> static int
>> store_killed_in_insn (x, insn)
>>      rtx x, insn;
>> {
>>   rtx set;
>>   if (!INSN_P (insn))
>>     return 0;
>>  
>>   if (GET_CODE (insn) == CALL_INSN)
>>     {
>>         return 1;
>>     }
>>   if (volatile_insn_p (PATTERN (insn)))
>>      return 1;
>>   if (multiple_sets (insn))
>>      return 1;
>>   set = single_set (insn);
>>   if (set)
>>     {
>>       /* Check for memory stores to aliased objects.  */
>>       if (GET_CODE (SET_DEST (set)) == MEM && !expr_equiv_p (SET_DEST (set), x))
>>         {
>>         if (find_loads (SET_DEST (set), x))
>>           return 1;
>>         }
>>       return find_loads (SET_SRC (set), x);
>>     }
>>   else
>>     return find_loads (PATTERN (insn), x);
>> }
>> 
>> (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.
>> 
>> 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)
> I duno, but as store motion is exciting thing, can you please send me the
> patch you use and tell me what exactly fail so I can try to investigate it?

Sure.
Coming soon.
I just want to see if i can't get it myself, since i'm getting much
closer than i was before.

There was also a cute thing we missed caused by the fact that we get

(set (reg:CC 75) (compare:CC (mem:BLK reg:SI 80) (mem:BLK reg:SI
79))),
and we never expected compare to have mem operands (and thus, never
checked the operands for aliased loads/stores)


I now have the redundant store elimination portion working perfectly
with no regressions.


> 
> I would love to see that beast working, so we will be able to kill
> mem hoisting in loop optimizer for instance.  It is the important
> amount of loop.c code and I would love to see the code motion in loop.c
> dead, as it de-facto isn't doing anything more than gcse/pre/load/store motion
> should.
> > So store_killed_in_insn isn't quite right, even in the above version.
> I will try to take a look if I can come with something more specific.
> 
> 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

-- 
"When I have a kid, I want to buy one of those strollers for
twins.  Then put the kid in and run around, looking frantic.
When he gets older, I'd tell him he used to have a brother, but
he didn't obey.
"-Steven Wright