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

Jush Lu jush.msn at gmail.com
Thu Aug 2 19:43:45 PDT 2012


Thanks for review.  Committed revision 161230.

Jush

On Thu, Aug 2, 2012 at 10:41 PM, Chad Rosier <mcrosier at apple.com> wrote:

> Please apply with those changes.
>
>  Chad
>
>
>
> On Aug 2, 2012, at 1:33 AM, Jush Lu <jush.msn at gmail.com> wrote:
>
>
> *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
>>
>>
>>
> <fast-isel-shifter.patch>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20120803/2df860c8/attachment.html>


More information about the llvm-commits mailing list