LGTM On Wed, Aug 21, 2013 at 1:51 PM, Logan Chien <tzuhsiang.chien at gmail.com> wrote:> Hi, > > I have created a workaround to deal with the PIC function call. With this > patch, the FastISel will switch back to DAG lowering mechanism if (1) there > is a function call in the basic block and (2) the relocation model is PIC. > Please have a look. Hoping the patch will help. > > Sincerely, > Logan > > > On Wed, Aug 21, 2013 at 10:17 AM, Gordon Keiser <gkeiser at arxan.com> wrote: >> >> Cool, I'll file a bug tomorrow at work and add you to the CC list. >> >> Thanks! >> Gordon Keiser >> Software Development Engineer >> Arxan Technologies >> gkeiser at arxan.com www.arxan.com >> Protecting the App EconomyT >> >> > -----Original Message----- >> > From: Eric Christopher [mailto:echristo at gmail.com] >> > Sent: Tuesday, August 20, 2013 9:47 PM >> > To: Gordon Keiser >> > Cc: llvmdev at cs.uiuc.edu; JF Bastien >> > Subject: Re: [LLVMdev] Broken PLT on ARM from R183966 >> > >> > Filing a bug would be a good start, go ahead and cc me and >> > jfb at google.com. >> > >> > Thanks! >> > >> > -eric >> > >> > On Tue, Aug 20, 2013 at 6:10 PM, Gordon Keiser <gkeiser at arxan.com> >> > wrote: >> > > For ARM targets on linux, revision 183966 made Fast ISel default. >> > > Unfortunately, Fast ISel is broken in terms of applying the >> > > ARMII::MO_PLT flags to calls in PIC mode (at least when emitting >> > > assembly); it never does this. The normal ISel pass handles this >> > > situation correctly so a temporary local change to disable FastISel >> > > for linux / NaCl targets is working for me right now. >> > > >> > > >> > > >> > > I'm not very familiar with the ISel passes. I'm guessing the correct >> > > thing to do here would be to apply the attribute correctly in FastISel >> > > so it works, but I'm kind of unfamiliar with this part of the code and >> > > won't have time to dig into it right now, although I'm willing to do >> > > it if someone points me to the right area of code. >> > > >> > > >> > > >> > > In the meantime, would it be worthwhile to submit in a modification to >> > > disable FastISel for non-darwin targets, or is it likely to be fixed >> > > quickly? >> > > >> > > >> > > >> > > -Gordon >> > > >> > > >> > > _______________________________________________ >> > > LLVM Developers mailing list >> > > LLVMdev at cs.uiuc.edu http://llvm.cs.uiuc.edu >> > > http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev >> > > >> >> _______________________________________________ >> LLVM Developers mailing list >> LLVMdev at cs.uiuc.edu http://llvm.cs.uiuc.edu >> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev > > > > _______________________________________________ > LLVM Developers mailing list > LLVMdev at cs.uiuc.edu http://llvm.cs.uiuc.edu > http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev >-- With best regards, Anton Korobeynikov Faculty of Mathematics and Mechanics, Saint Petersburg State University
lgtm On Wed, Aug 21, 2013 at 3:18 AM, Anton Korobeynikov <anton at korobeynikov.info> wrote:> LGTM > > On Wed, Aug 21, 2013 at 1:51 PM, Logan Chien <tzuhsiang.chien at gmail.com> > wrote: > > Hi, > > > > I have created a workaround to deal with the PIC function call. With > this > > patch, the FastISel will switch back to DAG lowering mechanism if (1) > there > > is a function call in the basic block and (2) the relocation model is > PIC. > > Please have a look. Hoping the patch will help. > > > > Sincerely, > > Logan > > > > > > On Wed, Aug 21, 2013 at 10:17 AM, Gordon Keiser <gkeiser at arxan.com> > wrote: > >> > >> Cool, I'll file a bug tomorrow at work and add you to the CC list. > >> > >> Thanks! > >> Gordon Keiser > >> Software Development Engineer > >> Arxan Technologies > >> gkeiser at arxan.com www.arxan.com > >> Protecting the App EconomyT > >> > >> > -----Original Message----- > >> > From: Eric Christopher [mailto:echristo at gmail.com] > >> > Sent: Tuesday, August 20, 2013 9:47 PM > >> > To: Gordon Keiser > >> > Cc: llvmdev at cs.uiuc.edu; JF Bastien > >> > Subject: Re: [LLVMdev] Broken PLT on ARM from R183966 > >> > > >> > Filing a bug would be a good start, go ahead and cc me and > >> > jfb at google.com. > >> > > >> > Thanks! > >> > > >> > -eric > >> > > >> > On Tue, Aug 20, 2013 at 6:10 PM, Gordon Keiser <gkeiser at arxan.com> > >> > wrote: > >> > > For ARM targets on linux, revision 183966 made Fast ISel default. > >> > > Unfortunately, Fast ISel is broken in terms of applying the > >> > > ARMII::MO_PLT flags to calls in PIC mode (at least when emitting > >> > > assembly); it never does this. The normal ISel pass handles this > >> > > situation correctly so a temporary local change to disable FastISel > >> > > for linux / NaCl targets is working for me right now. > >> > > > >> > > > >> > > > >> > > I'm not very familiar with the ISel passes. I'm guessing the > correct > >> > > thing to do here would be to apply the attribute correctly in > FastISel > >> > > so it works, but I'm kind of unfamiliar with this part of the code > and > >> > > won't have time to dig into it right now, although I'm willing to do > >> > > it if someone points me to the right area of code. > >> > > > >> > > > >> > > > >> > > In the meantime, would it be worthwhile to submit in a modification > to > >> > > disable FastISel for non-darwin targets, or is it likely to be fixed > >> > > quickly? > >> > > > >> > > > >> > > > >> > > -Gordon > >> > > > >> > > > >> > > _______________________________________________ > >> > > LLVM Developers mailing list > >> > > LLVMdev at cs.uiuc.edu http://llvm.cs.uiuc.edu > >> > > http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev > >> > > > >> > >> _______________________________________________ > >> LLVM Developers mailing list > >> LLVMdev at cs.uiuc.edu http://llvm.cs.uiuc.edu > >> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev > > > > > > > > _______________________________________________ > > LLVM Developers mailing list > > LLVMdev at cs.uiuc.edu http://llvm.cs.uiuc.edu > > http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev > > > > > > -- > With best regards, Anton Korobeynikov > Faculty of Mathematics and Mechanics, Saint Petersburg State University > _______________________________________________ > LLVM Developers mailing list > LLVMdev at cs.uiuc.edu http://llvm.cs.uiuc.edu > http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev >-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20130821/10a96792/attachment.html>
Hi Anton and JF, Thanks for your review. After reading the source code more carefully, I have come up with a different way fix this issue. We can simply resolve this issue by adding ARMII::MO_PLT flags with MachineInstrBuilder in FastISel pass (without failing back to DAG lowering). The new patch is attached, and the test case is not changed. Sorry for your inconvenience. Please have a look. Thanks for your help. Sincerely, Logan On Wed, Aug 21, 2013 at 10:52 PM, JF Bastien <jfb at google.com> wrote:> lgtm > > > On Wed, Aug 21, 2013 at 3:18 AM, Anton Korobeynikov < > anton at korobeynikov.info> wrote: > >> LGTM >> >> On Wed, Aug 21, 2013 at 1:51 PM, Logan Chien <tzuhsiang.chien at gmail.com> >> wrote: >> > Hi, >> > >> > I have created a workaround to deal with the PIC function call. With >> this >> > patch, the FastISel will switch back to DAG lowering mechanism if (1) >> there >> > is a function call in the basic block and (2) the relocation model is >> PIC. >> > Please have a look. Hoping the patch will help. >> > >> > Sincerely, >> > Logan >> > >> > >> > On Wed, Aug 21, 2013 at 10:17 AM, Gordon Keiser <gkeiser at arxan.com> >> wrote: >> >> >> >> Cool, I'll file a bug tomorrow at work and add you to the CC list. >> >> >> >> Thanks! >> >> Gordon Keiser >> >> Software Development Engineer >> >> Arxan Technologies >> >> gkeiser at arxan.com www.arxan.com >> >> Protecting the App EconomyT >> >> >> >> > -----Original Message----- >> >> > From: Eric Christopher [mailto:echristo at gmail.com] >> >> > Sent: Tuesday, August 20, 2013 9:47 PM >> >> > To: Gordon Keiser >> >> > Cc: llvmdev at cs.uiuc.edu; JF Bastien >> >> > Subject: Re: [LLVMdev] Broken PLT on ARM from R183966 >> >> > >> >> > Filing a bug would be a good start, go ahead and cc me and >> >> > jfb at google.com. >> >> > >> >> > Thanks! >> >> > >> >> > -eric >> >> > >> >> > On Tue, Aug 20, 2013 at 6:10 PM, Gordon Keiser <gkeiser at arxan.com> >> >> > wrote: >> >> > > For ARM targets on linux, revision 183966 made Fast ISel default. >> >> > > Unfortunately, Fast ISel is broken in terms of applying the >> >> > > ARMII::MO_PLT flags to calls in PIC mode (at least when emitting >> >> > > assembly); it never does this. The normal ISel pass handles this >> >> > > situation correctly so a temporary local change to disable FastISel >> >> > > for linux / NaCl targets is working for me right now. >> >> > > >> >> > > >> >> > > >> >> > > I'm not very familiar with the ISel passes. I'm guessing the >> correct >> >> > > thing to do here would be to apply the attribute correctly in >> FastISel >> >> > > so it works, but I'm kind of unfamiliar with this part of the code >> and >> >> > > won't have time to dig into it right now, although I'm willing to >> do >> >> > > it if someone points me to the right area of code. >> >> > > >> >> > > >> >> > > >> >> > > In the meantime, would it be worthwhile to submit in a >> modification to >> >> > > disable FastISel for non-darwin targets, or is it likely to be >> fixed >> >> > > quickly? >> >> > > >> >> > > >> >> > > >> >> > > -Gordon >> >> > > >> >> > > >> >> > > _______________________________________________ >> >> > > LLVM Developers mailing list >> >> > > LLVMdev at cs.uiuc.edu http://llvm.cs.uiuc.edu >> >> > > http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev >> >> > > >> >> >> >> _______________________________________________ >> >> LLVM Developers mailing list >> >> LLVMdev at cs.uiuc.edu http://llvm.cs.uiuc.edu >> >> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev >> > >> > >> > >> > _______________________________________________ >> > LLVM Developers mailing list >> > LLVMdev at cs.uiuc.edu http://llvm.cs.uiuc.edu >> > http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev >> > >> >> >> >> -- >> With best regards, Anton Korobeynikov >> Faculty of Mathematics and Mechanics, Saint Petersburg State University >> _______________________________________________ >> LLVM Developers mailing list >> LLVMdev at cs.uiuc.edu http://llvm.cs.uiuc.edu >> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev >> > >-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20130821/3d4bce48/attachment.html> -------------- next part -------------- A non-text attachment was scrubbed... Name: 0001-Fix-ARM-FastISel-PIC-function-call.patch Type: application/octet-stream Size: 2331 bytes Desc: not available URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20130821/3d4bce48/attachment.obj>