[llvm-commits] Add support for shl, lshr, and ashr instructions

Jush Lu jush.msn at gmail.com
Thu Aug 2 01:33:49 PDT 2012


*Hi,

Attachment is new patch, basically I use the test cases you gave in last
email, I just add few lines for FileCheck.

I think falling back to selection DAG isel is really better way when the
shift amount is zero or greater than the width of the value type, so I
modified code for this.

I also modified code as Jim’ suggestion.

Thanks,
Jush *


On Thu, Aug 2, 2012 at 2:57 AM, Jim Grosbach <grosbach at apple.com> wrote:

>
> On Aug 1, 2012, at 11:18 AM, Chad Rosier <mcrosier at apple.com> wrote:
>
> Jush,
> In the case that the shift amount is zero or greater then the width of the
> value type, should we just fall back to selection DAG isel to ensure we
> have consistent behavior (in the event that selection DAG isel changes)?
>  Perhaps someone else could comment on this.
>
>
>
> This seems reasonable to me.
>
> Additional minor thing:
>
> +  if (isa<ConstantInt>(Src2Value)) {
> +    Opc = ARM::MOVsi;
> +    const ConstantInt *CI = cast<ConstantInt>(Src2Value);
>
> This can be simplified to:
> if (const ConstantInt *CI = dyn_cast<ConstantInt>(Src2Value)) {
>   Opc = ARM::MOVsi;
>
>
> -Jim
>
>
> My only other suggestion would be to simplify the test cases.  For example,
>
> define i32 @shl() nounwind ssp {
> entry:
>   %shl = shl i32 -1, 2
>   ret i32 %shl
> }
>
> define i32 @shl_reg(i32 %src1, i32 %src2) nounwind ssp {
> entry:
>   %shl = shl i32 %src1, %src2
>   ret i32 %shl
> }
>
> define i32 @lshr() nounwind ssp {
> entry:
>   %lshr = lshr i32 -1, 2
>   ret i32 %lshr
> }
>
> define i32 @lshr_reg(i32 %src1, i32 %src2) nounwind ssp {
> entry:
>   %lshr = lshr i32 %src1, %src2
>   ret i32 %lshr
> }
>
> define i32 @ashr() nounwind ssp {
> entry:
>   %ashr = ashr i32 -1, 2
>   ret i32 %ashr
> }
>
> define i32 @ashr_reg(i32 %src1, i32 %src2) nounwind ssp {
> entry:
>   %ashr = ashr i32 %src1, %src2
>   ret i32 %ashr
> }
>
> Basically, I ran mem2reg on your test cases… :)
>
> While I realize this isn't the type of code fast-isel is going to
> generate, this is much more concise in terms of what you're trying to test
> (and that's all we care about).
>
> Please apply after updating the test cases.  If anyone disagrees with the
> handing of the zero/32 shift we can handle that as a post commit; I'm ok
> with leaving as is..
>
>  Chad
>
>
> On Jul 31, 2012, at 11:05 PM, Jush Lu wrote:
>
> *Hi
>
> This patch makes ARMFastISel able to handle shl, lshr, and ashr
> instructions, please help me review it, thanks.
>
> Jush* <fast-isel-shifter.patch>
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20120802/a0976b31/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: fast-isel-shifter.patch
Type: application/octet-stream
Size: 3904 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20120802/a0976b31/attachment.obj>


More information about the llvm-commits mailing list