<div dir="auto">Hi, do you expect that you'll be able to make my suggested changes any time soon? I can take the patch over if you'd like; I think I'll need something like it to support one of my upcoming Clang changes.<div dir="auto"><br></div><div dir="auto">Peter</div></div><div class="gmail_extra"><br><div class="gmail_quote">On Nov 28, 2016 2:33 PM, "Peter Collingbourne via Phabricator" <<a href="mailto:reviews@reviews.llvm.org">reviews@reviews.llvm.org</a>> wrote:<br type="attribution"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">pcc added inline comments.<br>
<br>
<br>
================<br>
Comment at: lib/Transforms/IPO/<wbr>WholeProgramDevirt.cpp:386<br>
<br>
+Optional<unsigned><br>
+DevirtModule::getOpAtOffset(<wbr>const Constant *I, const uint64_t Offset) {<br>
----------------<br>
Could be simpler to return a `const Constant *` from this function.<br>
<br>
<br>
================<br>
Comment at: lib/Transforms/IPO/<wbr>WholeProgramDevirt.cpp:428-429<br>
<br>
-    auto Init = dyn_cast<ConstantArray>(TM.<wbr>Bits->GV->getInitializer());<br>
-    if (!Init)<br>
-      return false;<br>
-    ArrayType *VTableTy = Init->getType();<br>
-<br>
-    uint64_t ElemSize =<br>
-        M.getDataLayout().<wbr>getTypeAllocSize(VTableTy-><wbr>getElementType());<br>
-    uint64_t GlobalSlotOffset = TM.Offset + ByteOffset;<br>
-    if (GlobalSlotOffset % ElemSize != 0)<br>
-      return false;<br>
+    const Constant *I = TM.Bits->GV->getInitializer();<br>
+    const uint64_t GlobalSlotOffset = TM.Offset + ByteOffset;<br>
+    Optional<unsigned> Op = getOpAtOffset(I, GlobalSlotOffset);<br>
----------------<br>
These two could be folded into the only use (after applying my suggestion above).<br>
<br>
<br>
================<br>
Comment at: test/Transforms/<wbr>WholeProgramDevirt/non-array-<wbr>vtable.ll:1<br>
 ; RUN: opt -S -wholeprogramdevirt %s | FileCheck %s<br>
<br>
----------------<br>
struct-vtable.ll would be a better name for this test.<br>
<br>
<br>
<a href="https://reviews.llvm.org/D26581" rel="noreferrer" target="_blank">https://reviews.llvm.org/<wbr>D26581</a><br>
<br>
<br>
<br>
</blockquote></div></div>