[PATCH] D133426: [LICM][LSR] Address a couple of special cases involving NULL.

Stefan Pintilie via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Sep 8 12:17:43 PDT 2022


stefanp added inline comments.


================
Comment at: llvm/lib/Transforms/Scalar/LICM.cpp:2101
+        }
+        return false; // Not a load or store and not null.
+      }
----------------
nikic wrote:
> So, the null special case doesn't make a lot of sense to me, in that promoting a null pointer is pretty pointless -- those accesses are UB anyway, right?
> 
> However, I do think that that this bailout can be removed entirely: https://reviews.llvm.org/D133485
> So, the null special case doesn't make a lot of sense to me, in that promoting a null pointer is pretty pointless -- those accesses are UB anyway, right?
> 
> However, I do think that that this bailout can be removed entirely: https://reviews.llvm.org/D133485

I took a look at that patch and I agree that is a better solution than what I have here.
I was trying to be very conservative and make changes only for NULL values but if the bailout can be removed completely that's much better.

I will change this patch to remove the changes in LICM.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D133426/new/

https://reviews.llvm.org/D133426



More information about the llvm-commits mailing list