[PATCH] D68328: Fix occurrences that size and range of pointers are assumed to be the same.

Thu Oct 3 06:26:29 PDT 2019

delena added inline comments.

================
Comment at: llvm/test/Transforms/InstCombine/icmp-custom-dl.ll:11
 ; CHECK-NEXT:    [[GEP1:%.*]] = getelementptr inbounds i8, i8* [[FOO:%.*]], i32 8
-; CHECK-NEXT:    [[TMP1:%.*]] = ptrtoint i8* [[GEP1]] to i32
-; CHECK-NEXT:    [[USE:%.*]] = zext i32 [[TMP1]] to i64
+; CHECK-NEXT:    [[TMP1:%.*]] = ptrtoint i8* [[GEP1]] to i40
+; CHECK-NEXT:    [[USE:%.*]] = zext i40 [[TMP1]] to i64
----------------
Joe wrote:
> delena wrote:
> > This change does not work for us. 
> > Doing this change we added a way to specify type for "ptrtoint" transformation and for size of GEP index. If you do not specify the index type, this change should be NFC. What exactly does not work for you?
> I don't believe that size of GEP index and type for "ptrtoint" transformation are the same thing. At least, that's how I interpreted langref: // 'The ‘ptrtoint’ instruction converts value to integer type ty2 by interpreting the pointer value as an integer'//. 
> 
> InstCombine was transforming 'ptrtoint i8* to i64' to 'ptrtoint i8* to i32', followed by a zext. Here, where the pointer size is 40 bits, that would mean 8 bits have been chopped off.
> 
> If you do not specify the index type, this change is NFC because index size defaults to pointer size.
I agree that we loose 8 bits in this case. Our promblem is in  i40 which is not an integer and we do not have any arithmetic ISA for i40.
Your current changes may cause many problems in our (out-of-tree) compiler.
Now we all have about 3 weeks local hollidays and we'd like to check your changes interlally when we back. So we whould ask you to hold on with this fix meanwhile.

Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68328/new/

https://reviews.llvm.org/D68328