[PATCH] D115387: [instcombine] Canonicalize constant index type to i64 for extractelement/insertelement

Thu Dec 9 11:04:51 PST 2021

craig.topper added a comment.

In D115387#3183352 <https://reviews.llvm.org/D115387#3183352>, @reames wrote:

> In D115387#3183282 <https://reviews.llvm.org/D115387#3183282>, @craig.topper wrote:
>
>> In D115387#3183235 <https://reviews.llvm.org/D115387#3183235>, @reames wrote:
>>
>>> Sounds like the consensus is i32?  I'll give it another day for further discussion, and will then update the patch if that's the direction we want to go in.
>>
>> Would we update IRBuilder to use i32 and perhaps change the interface from uint64_t to unsigned?
>
> Can we not increase the scope of the patch?  This is purely a minor cleanup as far as I'm concerned, and if it grows too much, I'll simply abandon it as not worth the effort.
>
> I am particularly concerns about changing argument types of the public API given possibility for silent downcast.

I guess my point was that it seems silly to have IRBuilder make something that InstCombine will always change. So maybe that's a vote for using i64 and not i32?

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115387