<div dir="ltr">Hi Arnold,<div><br></div><div style>I like the approach, and I think we could do the same for all types of instructions, not just load.</div><div style><br></div><div style>Regarding this specific code, I have some comments...<br>
</div><div style><br></div><div style>1. You seem to be cramming load & stores with a lot of non-shared code. Maybe it'd be more efficient to have a MemoryCostComp that both LoadCostComp and StoreCostComp derive and implement the specific parts.</div>
<div style><br></div><div style>2. Assert Liberally!</div><div style><br></div><div style><div>+    // It better be a load now.</div><div>+    Load = cast<LoadInst>(MemInst);</div><div><br></div><div style>to </div>
<div style><br></div><div style><div>+    Load = dyn_cast<LoadInst>(MemInst);</div><div>+    assert(Load && "Invalid memory instruction type");</div><div><br></div><div style>or use llvm_unreachable().</div>
<div style><br></div><div style>3. All this can be omitted if you use Instr* instead of specific LoadInst/StoreInst and if you common up code in a base class:</div><div style><br></div><div style><div>+  unsigned getOpcode() {</div>
<div>+    if (Store)</div><div>+      return Store->getOpcode();</div><div>+</div><div>+    return Load->getOpcode();</div><div>+  }</div><div>+</div><div>+  unsigned getAlignment() {</div><div>+    if (Store)</div>
<div>+      return Store->getAlignment();</div><div>+</div><div>+    return Load->getAlignment();</div><div>+  }</div><div>+</div><div>+  unsigned getPointerAddressSpace() {</div><div>+    if (Store)</div><div>+      return Store->getPointerAddressSpace();</div>
<div>+</div><div>+    return Load->getPointerAddressSpace();</div><div>+  }</div></div><div><br></div><div><br></div></div></div><div class="gmail_extra" style><br>cheers,</div><div class="gmail_extra" style>--renato</div>
</div>