support for division by constant in APInt

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
7 messages Options
Reply | Threaded
Open this post in threaded view
|

support for division by constant in APInt

Jay Foad-2
In lib/CodeGen/SelectionDAG/TargetLowering.cpp there are some
functions magic() and magicu() that support optimising division by a
constant. I'd like to use these functions in an LLVM FunctionPass that
I'm working on. The attached patch moves these functions out of
TargetLowering.cpp and into the APInt class, so that I can reuse them
in my pass. What do you think?

It looks to me like these functions were copied straight out of
"Hacker's Delight", Henry S. Warren, Jr., so I added a comment
acknowledging that.

Thanks,
Jay.

_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev

patch.magic (12K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: support for division by constant in APInt

Chris Lattner-2

On Apr 23, 2009, at 6:26 AM, Jay Foad wrote:

> In lib/CodeGen/SelectionDAG/TargetLowering.cpp there are some
> functions magic() and magicu() that support optimising division by a
> constant. I'd like to use these functions in an LLVM FunctionPass that
> I'm working on. The attached patch moves these functions out of
> TargetLowering.cpp and into the APInt class, so that I can reuse them
> in my pass. What do you think?
>
> It looks to me like these functions were copied straight out of
> "Hacker's Delight", Henry S. Warren, Jr., so I added a comment
> acknowledging that.

The patch looks fine to me, does it pass regression tests etc?  If so,  
please apply, or contact me off list if you don't yet have commit  
access.  Thanks Jay!

-Chris
_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
Reply | Threaded
Open this post in threaded view
|

Re: support for division by constant in APInt

Jay Foad-2
> The patch looks fine to me, does it pass regression tests etc?

Yes, I've just run a successful "make" in the test-suite module. It
took hours! Is there a smaller set of regression tests I could run for
changes like this in future?

Thanks,
Jay.
_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
Reply | Threaded
Open this post in threaded view
|

Re: support for division by constant in APInt

Misha Brukman-3
On Mon, Apr 27, 2009 at 9:45 AM, Jay Foad <[hidden email]> wrote:
> The patch looks fine to me, does it pass regression tests etc?

Yes, I've just run a successful "make" in the test-suite module. It
took hours! Is there a smaller set of regression tests I could run for
changes like this in future?

"make unittests" is a smaller set of tests, but given that it doesn't have particularly high coverage, it shouldn't be the only tests you run.

There are unittests for APInt, but they wouldn't cover a refactoring change such as this one.  Please feel free to add a unittest with your change.

_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
Reply | Threaded
Open this post in threaded view
|

Re: support for division by constant in APInt

Tanya Lattner-2

On Apr 27, 2009, at 11:33 AM, Misha Brukman wrote:

On Mon, Apr 27, 2009 at 9:45 AM, Jay Foad <[hidden email]> wrote:
> The patch looks fine to me, does it pass regression tests etc?

Yes, I've just run a successful "make" in the test-suite module. It
took hours! Is there a smaller set of regression tests I could run for
changes like this in future?

"make unittests" is a smaller set of tests, but given that it doesn't have particularly high coverage, it shouldn't be the only tests you run.

There are unittests for APInt, but they wouldn't cover a refactoring change such as this one.  Please feel free to add a unittest with your change.

'make check' runs regression tests. 

You can also run a subset of llvm-test (test-suite), but its always better if you can run all of them.

-Tanya


_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev


_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
Reply | Threaded
Open this post in threaded view
|

Re: support for division by constant in APInt

Jay Foad-2
> 'make check' runs regression tests.

Thanks - that's what I was missing! For what it's worth, my patch also
passes these regression tests.

Thanks,
Jay.
_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
Reply | Threaded
Open this post in threaded view
|

Re: support for division by constant in APInt

Jay Foad-2
In reply to this post by Chris Lattner-2
> The patch looks fine to me, does it pass regression tests etc?  If so,
> please apply, or contact me off list if you don't yet have commit
> access.  Thanks Jay!

Committed here:

http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20090427/076916.html

Thanks!
Jay.

_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev