Handle big index in getelementptr instruction for AArch64FastISel

[chfast]

Summary

This is a fix http://reviews.llvm.org/D8219 applied also to AArch64FastISel, as it has a copy of the code fixed previously.

[t.p.northover]

The code looks reasonable, but that test isn't going to work. It'll fail if those other backends aren't compiled in (lit.local.cfg explicitly requires only X86 in the test/CodeGen/X86 directory). Your best bet is probably to copy it into each target's subdirectory.

Cheers.

Tim.

[chilledheart]

Thank you for doing this fix, chfast.

I think we can probably copy this test into the directory of each target. Since the target R600 is still falling, we can add the part for R600 separately in a different patch.

[chilledheart]

Hi chfast and Northover:

It seems no one wants to fix the R600 backend part in a short time, so I filed the bug with R600 backend in the bugzilla for further reference. The bug's link is http://llvm.org/PR23312 .

Cheers,
Chilledheart

[chfast]

In D9140#159499, @chilledheart wrote:

Thank you for doing this fix, chfast.

I think we can probably copy this test into the directory of each target. Since the target R600 is still falling, we can add the part for R600 separately in a different patch.

I'm not convinced about adding the test for every target. I added for arm64 as it was failing and fix is for that target. I don't know any other target beside R600 that fails on that.

[chilledheart]

In D9140#159973, @chfast wrote:

In D9140#159499, @chilledheart wrote:

Thank you for doing this fix, chfast.

I think we can probably copy this test into the directory of each target. Since the target R600 is still falling, we can add the part for R600 separately in a different patch.

I'm not convinced about adding the test for every target. I added for arm64 as it was failing and fix is for that target. I don't know any other target beside R600 that fails on that.

Or we can add this test for other targets later.

[chfast]

Do we have a consensus about the tests for other targets?

[chfast]

Ping.

[chfast]

Ping.

[t.p.northover]

I think it's reasonable with just the AArch64 test added.

I'm not a massive fan of the way the test is written in general, but it might actually allow it to live in test/Generic once R600 is fixed.

Tim.

[chfast]

Are we doing something with that?

lib/Target/AArch64/AArch64FastISel.cpp

bool AArch64FastISel::selectGetElementPtr(const Instruction *I) {
   // Keep a running tab of the total offset to coalesce multiple N = N + Offset
   // into a single N = N + TotalOffset.
   uint64_t TotalOffs = 0;
   Type *Ty = I->getOperand(0)->getType();
   MVT VT = TLI.getPointerTy();
   for (auto OI = std::next(I->op_begin()), E = I->op_end(); OI != E; ++OI) {
     const Value *Idx = *OI;
     if (auto *StTy = dyn_cast<StructType>(Ty)) {
-      unsigned Field = cast<ConstantInt>(Idx)->getZExtValue();
+      uint64_t Field = cast<ConstantInt>(Idx)->getZExtValue();
       // N = N + Offset
       if (Field)
         TotalOffs += DL.getStructLayout(StTy)->getElementOffset(Field);
       Ty = StTy->getElementType(Field);
     } else {
       Ty = cast<SequentialType>(Ty)->getElementType();
       // If this is a constant subscript, handle it quickly.
       if (const auto *CI = dyn_cast<ConstantInt>(Idx)) {
         if (CI->isZero())
           continue;
         // N = N + Offset
-        TotalOffs +=
-            DL.getTypeAllocSize(Ty) * cast<ConstantInt>(CI)->getSExtValue();
+        uint64_t IdxN = CI->getValue().sextOrTrunc(64).getSExtValue();
+        TotalOffs += DL.getTypeAllocSize(Ty) * IdxN;
         continue;
       }
       if (TotalOffs) {
         N = emitAdd_ri_(VT, N, NIsKill, TotalOffs);
         if (!N)
           return false;
         NIsKill = true;
         TotalOffs = 0;

test/CodeGen/AArch64/getelementptr.ll

+; RUN: llc < %s -O0 -march=arm64
+; RUN: llc < %s -O2 -march=arm64
+
+; Test big index trunc to pointer size:
+
+define i8* @test_trunc65(i8* %ptr) nounwind {
+; CHECK-LABEL: test_trunc65
+; CHECK: 3
+  %d = getelementptr i8, i8* %ptr, i65 18446744073709551619 ; 2^64 + 3
+  ret i8* %d
+}
+
+define i8* @test_trunc128(i8* %ptr) nounwind {
+; CHECK-LABEL: test_trunc128
+; CHECK: 5
+  %d = getelementptr i8, i8* %ptr, i128 18446744073709551621 ; 2^64 + 5
+  ret i8* %d
+}
+
+define i8* @test_trunc160(i8* %ptr) nounwind {
+; CHECK-LABEL: test_trunc160
+; CHECK: 8
+  %d = getelementptr i8, i8* %ptr, i160 18446744073709551624 ; 2^64 + 8
+  ret i8* %d
+}
+
+define i8* @test_trunc256(i8* %ptr) nounwind {
+; CHECK-LABEL: test_trunc256
+; CHECK: 13
+  %d = getelementptr i8, i8* %ptr, i256 18446744073709551629 ; 2^64 + 13
+  ret i8* %d
+}
+
+define i8* @test_trunc2048(i8* %ptr) nounwind {
+; CHECK-LABEL: test_trunc2048
+; CHECK: 21
+  %d = getelementptr i8, i8* %ptr, i2048 18446744073709551637 ; 2^64 + 21
+  ret i8* %d
+}
+
+
+; Test small index sext to pointer size
+
+define i8* @test_sext3(i8* %ptr) nounwind {
+; CHECK-LABEL: test_sext3
+; CHECK: -3
+  %d = getelementptr i8, i8* %ptr, i3 -3
+  ret i8* %d
+}
+
+define i8* @test_sext5(i8* %ptr) nounwind {
+; CHECK-LABEL: test_sext5
+; CHECK: -5
+  %d = getelementptr i8, i8* %ptr, i5 -5
+  ret i8* %d
+}
+
+define i8* @test_sext8(i8* %ptr) nounwind {
+; CHECK-LABEL: test_sext8
+; CHECK: -8
+  %d = getelementptr i8, i8* %ptr, i8 -8
+  ret i8* %d
+}
+
+define i8* @test_sext13(i8* %ptr) nounwind {
+; CHECK-LABEL: test_sext13
+; CHECK: -13
+  %d = getelementptr i8, i8* %ptr, i8 -13
+  ret i8* %d
+}
+
+define i8* @test_sext16(i8* %ptr) nounwind {
+; CHECK-LABEL: test_sext16
+; CHECK: -21
+  %d = getelementptr i8, i8* %ptr, i8 -21
+  ret i8* %d
+}