[PATCH] [IR] Make {extract, insert}element accept an index of any integer type.

[Chris Lattner]

On Apr 26, 2014, at 8:09 PM, Michael Spencer <bigcheesegs at gmail.com> wrote:
>>> The movslq is uneeded, but is present because of the trunc to i32 and then
>>> sext back to i64 that the backend adds for vbroadcastss.
>>> 
>>> We can't remove it because it changes the meaning.
>> 
>> How does it change meaning?  Only the low 2 bits of the index are meaningful, if any of the other ones are set, you get an undefined result.
>> 
>> -Chris
> 
> if %j has any bits > 32 set to 1, the trunc removes them. At the
> IR/SDag level we don't know why the trunc is there (the intent may be
> to mask out the top bits), so we can't remove it.

Ok, so you're saying that we lose the information, not that we don't have enough information. :-)

I agree that this is the obviously right thing to do.

That said, your bitcode reader patch doesn't look right:

      Type *IdxTy = getTypeByID(Record[2]);
      if (!IdxTy || !IdxTy->isIntegerTy()) // FIXME: Remove with LLVM 4.0
        IdxTy = Type::getInt32Ty(Context);
      Constant *Op0 = ValueList.getConstantFwdRef(Record[1], OpTy);
      Constant *Op1 = ValueList.getConstantFwdRef(Record[2], IdxTy);

Unconditionally checking Record[2] as an integer type is not a reliable way to determine if you're dealing with an old format.  Instead, you should check Record.size() to see if you have the old format.

Also, please audit *everything* that touches insert and extractelement instructions.  There shouldn't be an overwhelming number of places, and I would not be surprised to see assumptions about the index types.  For example, we do insert to extract forwarding and probably assume that the index types match.  Further, how could this not require any selectiondag changes?

In order to test this, it is probably best to (temporarily) hack clang to start unconditionally producing i64 indexes, then run tests.

-Chris