Let me have a try. <div><br></div><div>The attached patch combines arm-jit-ConstantVector-ConstantArray.patch and arm-jit-LEApcrel.patch provided by Shih-Wei with typo/coding fixing mentioned by Nick. The test cases used in PR7226 and PR7227 are the same. And only with this combined patch, test case can be successfully JIT'ed and executed. I simplified the test case as below:<div>
<br></div><div>==== BEGIN TEST CASE SNIPPET ====</div><div><div>int gTextureSwap;</div><div>float scale[1];</div><div><br></div><div>int main(int argc, char** argv) {</div><div> if (gTextureSwap != 0) </div><div> scale[0] = .25f;</div>
<div> else </div><div> scale[0] = 4.f;</div><div><br></div><div> return 55;</div><div>}</div><div>==== END TEST CASE SNIPPET ====</div><div><br></div><div>The assembly showed after JIT is (by disassemble the result binary):</div>
<div><br></div><div><div>JIT: Disassembled code: main</div><div> e3060ff8 <span class="Apple-tab-span" style="white-space:pre"> </span>movw<span class="Apple-tab-span" style="white-space:pre"> </span>r0, #28664</div><div>
e34100ca <span class="Apple-tab-span" style="white-space:pre"> </span>movt<span class="Apple-tab-span" style="white-space:pre"> </span>r0, #4298</div><div> e3a02000 <span class="Apple-tab-span" style="white-space:pre"> </span>mov<span class="Apple-tab-span" style="white-space:pre"> </span>r2, #0</div>
<div> e5900000 <span class="Apple-tab-span" style="white-space:pre"> </span>ldr<span class="Apple-tab-span" style="white-space:pre"> </span>r0, [r0]</div><div> e3500000 <span class="Apple-tab-span" style="white-space:pre"> </span>cmp<span class="Apple-tab-span" style="white-space:pre"> </span>r0, #0</div>
<div> e3a00000 <span class="Apple-tab-span" style="white-space:pre"> </span>mov<span class="Apple-tab-span" style="white-space:pre"> </span>r0, #0</div><div> 3a00001 <span class="Apple-tab-span" style="white-space:pre"> </span>moveq<span class="Apple-tab-span" style="white-space:pre"> </span>r0, #1</div>
<div> e3500000 <span class="Apple-tab-span" style="white-space:pre"> </span>cmp<span class="Apple-tab-span" style="white-space:pre"> </span>r0, #0</div><div> 13a02004 <span class="Apple-tab-span" style="white-space:pre"> </span>movne<span class="Apple-tab-span" style="white-space:pre"> </span>r2, #4</div>
<div> e28f0018 <span class="Apple-tab-span" style="white-space:pre"> </span>add<span class="Apple-tab-span" style="white-space:pre"> </span>r0, pc, #24 ; require pseudo instruction LEApcrel</div><div> e0800002 <span class="Apple-tab-span" style="white-space:pre"> </span>add<span class="Apple-tab-span" style="white-space:pre"> </span>r0, r0, r2</div>
<div> ed900a00 <span class="Apple-tab-span" style="white-space:pre"> </span>vldr.32<span class="Apple-tab-span" style="white-space:pre"> </span>s0, [r0]</div><div> e3060ff4 <span class="Apple-tab-span" style="white-space:pre"> </span>movw<span class="Apple-tab-span" style="white-space:pre"> </span>r0, #28660</div>
<div> e34100ca <span class="Apple-tab-span" style="white-space:pre"> </span>movt<span class="Apple-tab-span" style="white-space:pre"> </span>r0, #4298</div><div> ed800a00 <span class="Apple-tab-span" style="white-space:pre"> </span>vstr.32<span class="Apple-tab-span" style="white-space:pre"> </span>s0, [r0]</div>
<div> e3a00037 <span class="Apple-tab-span" style="white-space:pre"> </span>mov<span class="Apple-tab-span" style="white-space:pre"> </span>r0, #55</div><div> e12fff1e <span class="Apple-tab-span" style="white-space:pre"> </span>bx<span class="Apple-tab-span" style="white-space:pre"> </span>lr</div>
<div> 3e800000 <span class="Apple-tab-span" style="white-space:pre"> </span>cdplo<span class="Apple-tab-span" style="white-space:pre"> </span>p0, #8, cr0, cr0, cr0, #0 ; 0x3e800000 = 0.25</div><div> 40800000 <span class="Apple-tab-span" style="white-space:pre"> </span>addmi<span class="Apple-tab-span" style="white-space:pre"> </span>r0, r0, r0 ; 0x40800000 = 4.0</div>
</div><div><br></div><div>In this case, as mentioned in Liao's bug report, 0.25f and 4.f in the code above will become ConstantArray of 2 elements. And I'm not sure whether there's any needed implementation for ConstantVector in emitConstantToMemory(...). To avoid to deal with the alignment and padding problems arising from materializing vector type, I just leave it out (I try to find a test case that will need vector constant materialization but no any result up to now). </div>
<div><br></div><div>However, here comes another question. Does anyone know what kind of "Constant" can be in emitConstantToMemory(...)?</div><div><br></div><div><br></div><div>Zonr</div><div><br></div><div><br>
<div class="gmail_quote">
On Thu, May 27, 2010 at 11:41 AM, Nick Lewycky <span dir="ltr"><<a href="mailto:nlewycky@google.com">nlewycky@google.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">
<div><div>Hi Shih-wei, I have a few comments:</div><div><br></div><div>+void ARMCodeEmitter::emitConstantToMemory(unsigned CPI, const Constant* C) {</div></div><div><br></div><div>The rest of the code uses "const Constant *C". Please place the space before the star.</div>
<div><br></div><div>In X86CodeEmitter, the </div><div><br></div><div>+ } else if (const ConstantVector *CV = dyn_cast<ConstantVector>(C)) {</div><div>+ for (unsigned i=0;i<CV->getNumOperands();i++)</div><div>
+ emitConstantToMemory(CPI, CV->getOperand(i));</div><div>+ } else if (const ConstantArray *CA = dyn_cast<ConstantArray>(C)) {</div><div>+ for (unsigned i=0;i<CA->getNumOperands();i++)</div><div>+ emitConstantToMemory(CPI, CA->getOperand(i));</div>
<div>+ } else {</div><div>+ llvm_unreachable("Unable to handle this constantpool entry!");</div><div>+ }</div><div><br></div><div>A few things. Firstly, please use proper whitespace, make your code look like the code around you. This means "for (unsigned i = 0; i < CA->getNumOperands(); i++)". Secondly, always hoist out the end computation of a loop, which would make it "for (unsigned i = 0, e = CV->getNumOperands(); i != e; i++)".</div>
<div><br></div><div>Now, you've added a lot of cases and I'm wondering whether it's time to switch away from the else-if nesting. As it is, the dyn_cast<> macros have to be tried in the order they're written in the code. Using a switch only evaluates the concrete type once, and makes the code:</div>
<div><br></div><div>switch (C->getValueID()) {</div><div> case GlobalVariableVal: // fall-through</div><div> case GlobalAliasVal: // fall-through</div><div> case FunctionVal: {</div><div> GlobalValue *GV = cast<GlobalValue>(C);</div>
<div> emitGlobalAddress ...</div><div> break;</div><div> }</div><div> case ConstantIntVal: {</div><div> ConstantInt *CI = cast<ConstantInt>(C);</div><div> ...</div><div> break;</div><div> }</div><div>
...</div><div>}</div><div><br></div><div>I also noticed that you've added potentially unbounded recursion (well, it's bounded by the depth of nesting of array types), but that doesn't look easy to fix, and it's probably not going to be a problem anyways (ie., we'd run out of ArrayType objects first).</div>
<div><br></div><div>Now finally, with all that said, I'm still not sure about this patch. Is the layout of a vector in memory the same as an array in memory? You're treating them the same with no padding or alignment issues considered, just stamping the elements into memory. Someone else should look at that before this patch lands.</div>
<div><br></div><font color="#888888"><div>Nick</div><div><br></div></font><div class="gmail_quote"><div><div></div><div class="h5">On 26 May 2010 02:27, Shih-wei Liao <span dir="ltr"><<a href="mailto:sliao@google.com" target="_blank">sliao@google.com</a>></span> wrote:<br>
</div></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div><div></div><div class="h5">
See <a href="http://llvm.org/bugs/show_bug.cgi?id=7226" target="_blank">http://llvm.org/bugs/show_bug.cgi?id=7226</a>: ARM JIT couldn't emit<br>
constant pool entry for "ConstantVector" and "ConstantArray".<br>
<br>
The patch passes both "make check" and "make check-lit". Basically,<br>
the patch added the processing of ConstantVector and ConstantArray.<br>
The problem is gone after the patch.<br>
--<br>
Thanks,<br>
<font color="#888888"> Shih-wei<br>
</font><br></div></div><div class="im">_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@cs.uiuc.edu" target="_blank">llvm-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
<br></div></blockquote></div><br>
<br>_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
<br></blockquote></div><br></div></div></div>