Re: [PATCH, rs6000] Add additional checks when identifying load/store instructions

[Pat Haugen via Gcc-patches]

On 8/6/21 11:05 AM, Segher Boessenkool wrote:
> On Fri, Aug 06, 2021 at 10:29:40AM -0500, Pat Haugen wrote:
>> On 8/6/21 10:02 AM, Segher Boessenkool wrote:
>>>> -  if (GET_CODE (pat) == SET)
>>>> +  if (GET_CODE (pat) == SET && REG_P (SET_DEST (pat)))
>>>>      return find_mem_ref (SET_SRC (pat), load_mem);
>>>
>>> So this now falls through if it is a SET of something else than a reg.
>>> Is that intentional?  If so, this should be in the changelog.
>>>
>>
>> Falling through eventually resulted in returning false, but no, wasn't 
>> really intentional to change the logic that way. I'll fix up both places to 
>> look like the following and resubmit.
>>
>>   if (GET_CODE (pat)== SET) 
>>     if (REG_P (SET_DEST (pat)))
>>       return find_mem_ref (SET_SRC (pat), load_mem);
>>     else
>>       return false;
> 
> Well, that isn't quiet it either, no?  It returns false for all stores
> then?

Yeah, I didn't word that clearly, meant I'd fix up both appropriately with the 
inner if-else logic.

Here's what it looks like now.

diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
index 279f00cc648..ad856254f52 100644
--- a/gcc/config/rs6000/rs6000.c
+++ b/gcc/config/rs6000/rs6000.c
@@ -18357,8 +18357,11 @@ is_load_insn1 (rtx pat, rtx *load_mem)
   if (!pat || pat == NULL_RTX)
     return false;
 
-  if (GET_CODE (pat) == SET)
-    return find_mem_ref (SET_SRC (pat), load_mem);
+  if (GET_CODE (pat)== SET)
+    if (REG_P (SET_DEST (pat)))
+      return find_mem_ref (SET_SRC (pat), load_mem);
+    else
+      return false;
 
   if (GET_CODE (pat) == PARALLEL)
     {
@@ -18395,7 +18398,10 @@ is_store_insn1 (rtx pat, rtx *str_mem)
     return false;
 
   if (GET_CODE (pat) == SET)
-    return find_mem_ref (SET_DEST (pat), str_mem);
+    if (REG_P (SET_SRC (pat)) || SUBREG_P (SET_SRC (pat)))
+      return find_mem_ref (SET_DEST (pat), str_mem);
+    else
+      return false;
 
   if (GET_CODE (pat) == PARALLEL)
     {

Unless I'm totally not understanding the point you're trying to get across to 
me.

-Pat