[llvm] r267120 - The following code would not work before this patch, due to the inability to take the address of a global object:

James Y Knight via llvm-commits llvm-commits at lists.llvm.org
Tue Apr 26 09:09:15 PDT 2016


Yes, this *definitely* needs a test case. A little bit of pseudo-code in
the commit message is not a useful test.

Also, please always use the proper syntax in your commit message to refer
to the review. You need to have a line like this: "Differential Revision:
http://reviews.llvm.org/D19368" so that it will get associated with the
review properly.

Finally, your commit message could be better. The first line should be a
short summary of what you've done. Then, expand upon that in later
paragraphs.

E.g.:
====
[SPARC] Fix DelaySlotFiller to handle ...whatever this fixes...

More lines of explanation why you need to do this. Blah blah
blah.

Differential Revision: http://reviews.llvm.org/D19368
====

On Fri, Apr 22, 2016 at 1:57 PM, Rafael EspĂ­ndola <
llvm-commits at lists.llvm.org> wrote:

> Why not add a testcase?
>
> Cheers,
> Rafael
>
>
> On 22 April 2016 at 04:13, Chris Dewhurst via llvm-commits
> <llvm-commits at lists.llvm.org> wrote:
> > Author: lerochris
> > Date: Fri Apr 22 03:13:47 2016
> > New Revision: 267120
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=267120&view=rev
> > Log:
> > The following code would not work before this patch, due to the
> inability to take the address of a global object:
> >
> > void func1() {
> >
> > ...
> > }
> >
> > int main(int argc, char** argv) {
> >
> > void (*pFunc)();
> > pFunc = &func1
> > pFunc();
> > ...
> > }
> >
> > Phabricator review: http://reviews.llvm.org/D19368
> >
> > Modified:
> >     llvm/trunk/lib/Target/Sparc/DelaySlotFiller.cpp
> >
> > Modified: llvm/trunk/lib/Target/Sparc/DelaySlotFiller.cpp
> > URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/Sparc/DelaySlotFiller.cpp?rev=267120&r1=267119&r2=267120&view=diff
> >
> ==============================================================================
> > --- llvm/trunk/lib/Target/Sparc/DelaySlotFiller.cpp (original)
> > +++ llvm/trunk/lib/Target/Sparc/DelaySlotFiller.cpp Fri Apr 22 03:13:47
> 2016
> > @@ -296,7 +296,7 @@ void Filler::insertCallDefsUses(MachineB
> >      RegUses.insert(Reg.getReg());
> >
> >      const MachineOperand &RegOrImm = MI->getOperand(1);
> > -    if (RegOrImm.isImm())
> > +    if (RegOrImm.isImm() || RegOrImm.isGlobal())
> >          break;
> >      assert(RegOrImm.isReg() && "CALLrr second operand is not a
> register.");
> >      assert(RegOrImm.isUse() && "CALLrr second operand is not a use.");
> >
> >
> > _______________________________________________
> > llvm-commits mailing list
> > llvm-commits at lists.llvm.org
> > http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160426/4f4c2a77/attachment.html>


More information about the llvm-commits mailing list