<html xmlns:v="urn:schemas-microsoft-com:vml" xmlns:o="urn:schemas-microsoft-com:office:office" xmlns:w="urn:schemas-microsoft-com:office:word" xmlns:m="http://schemas.microsoft.com/office/2004/12/omml" xmlns="http://www.w3.org/TR/REC-html40"><head><meta http-equiv=Content-Type content="text/html; charset=iso-2022-jp"><meta name=Generator content="Microsoft Word 14 (filtered medium)"><style><!--
/* Font Definitions */
@font-face
{font-family:SimSun;
panose-1:2 1 6 0 3 1 1 1 1 1;}
@font-face
{font-family:SimSun;
panose-1:2 1 6 0 3 1 1 1 1 1;}
@font-face
{font-family:Calibri;
panose-1:2 15 5 2 2 2 4 3 2 4;}
@font-face
{font-family:Tahoma;
panose-1:2 11 6 4 3 5 4 4 2 4;}
@font-face
{font-family:"\@SimSun";
panose-1:2 1 6 0 3 1 1 1 1 1;}
/* Style Definitions */
p.MsoNormal, li.MsoNormal, div.MsoNormal
{margin:0in;
margin-bottom:.0001pt;
font-size:12.0pt;
font-family:SimSun;
mso-fareast-language:ZH-CN;}
a:link, span.MsoHyperlink
{mso-style-priority:99;
color:blue;
text-decoration:underline;}
a:visited, span.MsoHyperlinkFollowed
{mso-style-priority:99;
color:purple;
text-decoration:underline;}
span.EmailStyle17
{mso-style-type:personal-reply;
font-family:"Calibri","sans-serif";
color:#1F497D;}
.MsoChpDefault
{mso-style-type:export-only;}
@page WordSection1
{size:8.5in 11.0in;
margin:1.0in 1.0in 1.0in 1.0in;}
div.WordSection1
{page:WordSection1;}
--></style><!--[if gte mso 9]><xml>
<o:shapedefaults v:ext="edit" spidmax="1026" />
</xml><![endif]--><!--[if gte mso 9]><xml>
<o:shapelayout v:ext="edit">
<o:idmap v:ext="edit" data="1" />
</o:shapelayout></xml><![endif]--></head><body lang=EN-US link=blue vlink=purple><div class=WordSection1><p class=MsoNormal><span style='font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D'>Why does this patch need to make a change to the ARMConstantIslandsPass? I think it may be because we are still using an arm instruction for the constant load instead of the equivalent thumb instruction (tLDRpci).<o:p></o:p></span></p><p class=MsoNormal><span style='font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D'><o:p> </o:p></span></p><p class=MsoNormal><span style='font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D'>I think your patch is incomplete for supporting thumb1. You need to modify the lowering for COPY_STRUCT_BYVAL_I32 in a few more places. We should be using the thumb1 opcodes for all the instructions generated by the lowering. You have handled the load/store case for the values, but you still need to take care of the constant pool load, the subtract (for the loop counter) and the branch. These instructions should use the tLDRpci (for constant pool load), tSubi8 (for the loop counter subtract), and tBcc (for the loop branch).<o:p></o:p></span></p><p class=MsoNormal><span style='font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D'><o:p> </o:p></span></p><p class=MsoNormal><span style='font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D'>I didn’t see your bug and had filed another bug for this same issue: <a href="http://llvm.org/bugs/show_bug.cgi?id=17309">http://llvm.org/bugs/show_bug.cgi?id=17309</a>. Please also check that your patch works with the test case I uploaded to that bug (also attached to this email). In addition to checks for thumb1, this test case also exposes a bug in the current code with the thumb2 lowering. It will trigger an assertion because an operand is added to a full instruction. It looks like the thumb2 failure comes from a copy/paste error when building the machine instructions.<o:p></o:p></span></p><p class=MsoNormal><span style='font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D'><o:p> </o:p></span></p><p class=MsoNormal><span style='font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D'><o:p> </o:p></span></p><p class=MsoNormal><span style='font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D;mso-fareast-language:EN-US'>-- </span><span style='font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D;mso-fareast-language:EN-US'>Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation<o:p></o:p></span></p><p class=MsoNormal><span style='font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D;mso-fareast-language:EN-US'><o:p> </o:p></span></p><p class=MsoNormal><span style='font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D'><o:p> </o:p></span></p><div style='border:none;border-left:solid blue 1.5pt;padding:0in 0in 0in 4.0pt'><div><div style='border:none;border-top:solid #B5C4DF 1.0pt;padding:3.0pt 0in 0in 0in'><p class=MsoNormal><b><span style='font-size:10.0pt;font-family:"Tahoma","sans-serif"'>From:</span></b><span style='font-size:10.0pt;font-family:"Tahoma","sans-serif"'> llvm-commits-bounces@cs.uiuc.edu [mailto:llvm-commits-bounces@cs.uiuc.edu] <b>On Behalf Of </b>???<br><b>Sent:</b> Monday, October 14, 2013 6:29 PM<br><b>To:</b> Manman Ren<br><b>Cc:</b> llvm-commits<br><b>Subject:</b> Re: review request Bug 16545 - COPY_STRUCT_BYVAL_I32 pseudo code generates unrecognized code<o:p></o:p></span></p></div></div><p class=MsoNormal><o:p> </o:p></p><div><p class=MsoNormal>I didn't add THUMB1 lines for function h simple armv5te doesn't support vldr instr.<o:p></o:p></p><div><p class=MsoNormal>And here is the patch for formatting.<o:p></o:p></p></div></div><div><p class=MsoNormal style='margin-bottom:12.0pt'><o:p> </o:p></p><div><p class=MsoNormal>2013/10/15 Manman Ren <<a href="mailto:manman.ren@gmail.com" target="_blank">manman.ren@gmail.com</a>><o:p></o:p></p><div><p class=MsoNormal>Hi,<o:p></o:p></p><div><p class=MsoNormal><o:p> </o:p></p></div><div><p class=MsoNormal>The changes to ARMISelLowering.cpp look good to me.<o:p></o:p></p></div><div><p class=MsoNormal><o:p> </o:p></p></div><div><div><p class=MsoNormal>--- lib/Target/ARM/ARMConstantIslandPass.cpp (<span lang=ZH-CN>版本</span> 187190)<o:p></o:p></p></div><div><p class=MsoNormal>+++ lib/Target/ARM/ARMConstantIslandPass.cpp (<span lang=ZH-CN>工作副本</span>)<o:p></o:p></p></div><div><p class=MsoNormal>@@ -634,6 +634,7 @@<o:p></o:p></p></div><div><p class=MsoNormal> initializeFunctionInfo(const std::vector<MachineInstr*> &CPEMIs) {<o:p></o:p></p></div><div><p class=MsoNormal> BBInfo.clear();<o:p></o:p></p></div><div><p class=MsoNormal> BBInfo.resize(MF->getNumBlockIDs());<o:p></o:p></p></div><div><p class=MsoNormal>+ const ARMSubtarget * Subtarget = &MF->getTarget().getSubtarget<ARMSubtarget>();<o:p></o:p></p></div><div><p class=MsoNormal> <o:p></o:p></p></div><div><p class=MsoNormal> // First thing, compute the size of all basic blocks, and see if the function<o:p></o:p></p></div><div><p class=MsoNormal> // has any inline assembly in it. If so, we have to be conservative about<o:p></o:p></p></div><div><p class=MsoNormal>@@ -757,7 +758,12 @@<o:p></o:p></p></div><div><p class=MsoNormal> case ARM::LDRi12:<o:p></o:p></p></div><div><p class=MsoNormal> case ARM::LDRcp:<o:p></o:p></p></div><div><p class=MsoNormal> case ARM::t2LDRpci:<o:p></o:p></p></div><div><p class=MsoNormal>- Bits = 12; // +-offset_12<o:p></o:p></p></div><div><p class=MsoNormal>+ if(Subtarget->hasV7Ops())<o:p></o:p></p></div><div><p class=MsoNormal>+ {<o:p></o:p></p></div><div><p class=MsoNormal>+ Bits = 12; // +-offset_12<o:p></o:p></p></div><div><p class=MsoNormal>+ } else {<o:p></o:p></p></div><div><p class=MsoNormal>+ Bits = 8; // +-offset_8<o:p></o:p></p></div><div><p class=MsoNormal>+ }<o:p></o:p></p></div></div><div><p class=MsoNormal><o:p> </o:p></p></div><div><p class=MsoNormal>Please simplify above by removing the parenthesis, or using "? 12 : 8".<o:p></o:p></p></div><div><p class=MsoNormal>I am not really familiar with the const island pass, so I cc'ed Bob for him to review it.<o:p></o:p></p></div><div><p class=MsoNormal><o:p> </o:p></p></div><div><p class=MsoNormal>The testing case looks good in general, is there any reason you didn't add THUMB1 lines for function h()?<o:p></o:p></p></div><div><p class=MsoNormal><o:p> </o:p></p></div><div><p class=MsoNormal>Thanks,<o:p></o:p></p></div><div><p class=MsoNormal>Manman<o:p></o:p></p></div><div><p class=MsoNormal><o:p> </o:p></p></div></div><div><p class=MsoNormal style='margin-bottom:12.0pt'><o:p> </o:p></p><div><p class=MsoNormal>On Sun, Oct 13, 2013 at 4:48 AM, <span lang=ZH-CN>林作健</span> <<a href="mailto:manjian2006@gmail.com" target="_blank">manjian2006@gmail.com</a>> wrote:<o:p></o:p></p><div><p class=MsoNormal><o:p> </o:p></p></div><p class=MsoNormal style='margin-bottom:12.0pt'><br>_______________________________________________<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><o:p></o:p></p></div><p class=MsoNormal><o:p> </o:p></p></div></div><p class=MsoNormal><o:p> </o:p></p></div></div></div></body></html>