[PATCH] D138645: [AAPointerInfo] rearrange code in preparation for further changes
    Sameer Sahasrabuddhe via Phabricator via llvm-commits 
    llvm-commits at lists.llvm.org
       
    Tue Nov 29 00:22:38 PST 2022
    
    
  
sameerds added inline comments.
================
Comment at: llvm/lib/Transforms/IPO/AttributorAttributes.cpp:1217
     return Changed;
   }
 
----------------
jdoerfert wrote:
> Isn't this now 25 identical lines and 2 different ones? What's the benefit here? Code duplication is bad.
I agree with that sentiment wholeheartedly. But D138646 that introduces multiple offsets has different changes for the two versions. We can hold this until that code is reviewed. If we find a readable way to keep a single version, that would be great.
================
Comment at: llvm/lib/Transforms/IPO/AttributorAttributes.cpp:1262
     return true;
   };
 
----------------
jdoerfert wrote:
> `Ty` can be nullptr, no? Did you test this?
At the two calls to this function, I don't see how it can be nullptr. What did I miss?
Repository:
  rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D138645/new/
https://reviews.llvm.org/D138645
    
    
More information about the llvm-commits
mailing list