[PATCH] D20262: [DSE]Split memset when the memset is small enough to be lowered to stores

Mehdi AMINI via llvm-commits llvm-commits at lists.llvm.org
Tue May 31 16:43:10 PDT 2016


mehdi_amini added a subscriber: mehdi_amini.

================
Comment at: lib/Transforms/Scalar/DeadStoreElimination.cpp:238
@@ +237,3 @@
+
+  if (MaxIntSize == 0)
+    MaxIntSize = 1;
----------------
junbuml wrote:
> qcolombet wrote:
> > junbuml wrote:
> > > dberlin wrote:
> > > > How and when can this happen?
> > > If there is no information about LegalIntWidth extracted from datalayout, this could be 0.  If we remove the target datalayout in input IR, this will be 0.
> > Like Daniel said, DL are mandatory now. If that may still happen, (int would be an illegal type??) add a comment on when this is the case and add a test case for that!
> As far as I check, I didn't encounter any complain when injecting an IR without "target datalayout" into opt. Also there are many test cases without datalayout. It seems also possible to assign empty string like target datalayout="". Did the mandatory mean in IRs generated from the frontend? 
> 
> If DL.getLargestLegalIntTypeSizeInBits() returns 0, I think we should bail out with the assumption that we don't have proper information about backend.
Having ` DL.getLargestLegalIntTypeSizeInBits()` returning 0 would mean that someone specifically asked for not having int types supported at all.
Handling this case means that LLVM acknowledges that it aims at supporting this case. Is it possible/desirable to support this?
Otherwise an assertion would be more appropriate IMO.


http://reviews.llvm.org/D20262





More information about the llvm-commits mailing list