[PATCH] D55934: Assertion in isAllocaPromotable due to extra bitcast goes into lifetime marker

[Chandler Carruth via Phabricator via llvm-commits]

chandlerc requested changes to this revision.
chandlerc added inline comments.
This revision now requires changes to proceed.


================
Comment at: lib/Transforms/Scalar/SROA.cpp:3036-3048
+    // For lifetime intrinsics it's ok to cast a pointer type of a new slice
+    // to a generic i8* pointer. There is a case when using the pointer type
+    // from whole alloca leads to an assertion in PromoteMemToReg. A code
+    // looks like this. The initial alloca is
+    //
+    // alloca [10 x float]
+    //
----------------
I think this comment can be much shorter and to the point.

Just say:

"""
Lifetime intrinsics always expect an i8* so directly get such a pointer for the new alloca slice.
"""

The rest is really talking about how you hit the issue and doesn't help the future reader IMO.


================
Comment at: test/Transforms/SROA/extra-cast-in-lifetime.ll:1-4
+; RUN: opt < %s -S -sroa | FileCheck %s
+
+ at MainKernel.callA = dso_local addrspace(3) global [10 x float] undef, align 4
+
----------------
Can you reduce this test further, for example by removing all the address spaces and using more standard compact names?

Also, could you simply append it to one of the existing tests? Probably just basictest.ll as there isn't really a good factored out test for lifetime markers.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D55934