[PATCH] D105485: [IR] Use a proper type for AttributeList indexing

[Guillaume Chatelet via Phabricator via llvm-commits]

gchatelet marked 2 inline comments as done.
gchatelet added subscribers: cherry, anna, thanm.
gchatelet added a comment.

@cherry @anna @thanm since you worked on D53602 <https://reviews.llvm.org/D53602>. Do you have a sense of whether the failing test is legitimate?

Basically these lines <https://github.com/llvm/llvm-project/blob/ba913b8da57dcdcda0572ec3a6b8d4e367f22803/llvm/test/Transforms/InstCombine/call-cast-attrs.ll#L21-L22> :

  call void bitcast (void (...)* @c to void (i32*)*)(i32* sret(i32) %y)
  call void bitcast (void (i32, ...)* @d to void (i32, i32*)*)(i32 0, i32* sret(i32) %y)

Are considered illegal by the Verifier <https://github.com/llvm/llvm-project/blob/ba913b8da57dcdcda0572ec3a6b8d4e367f22803/llvm/lib/IR/Verifier.cpp#L2258-L2263> and trigger the following assert:

  if (TargetFuncType->isVarArg()) {
    AttributeSet ArgAttrs = Attrs.getParamAttributes(5 + i);
    Assert(!ArgAttrs.hasAttribute(Attribute::StructRet),
           "Attribute 'sret' cannot be used for vararg call arguments!",
           Call);
  }

Any help here would be highly appreciated!



================
Comment at: llvm/include/llvm/IR/Attributes.h:444
+  static Index getArgIndex(unsigned ArgNo) { return Index::fromArgNo(ArgNo); }
+  static const Index ReturnIndex;
+  static const Index FunctionIndex;
----------------
courbet wrote:
> `constexpr` ?
As discussed offline, I wasn't able to do so because the definition of `AttributeList::Index::Index()` is not available before `AttributeList` class is finished parsing.
This then fires the following error `expression must have a constant value -- constexpr function "llvm::AttributeList::Index::Index(unsigned int Value)" (declared at line 394) is not definedC/C++(28)`


================
Comment at: llvm/lib/Transforms/Utils/AssumeBundleBuilder.cpp:212
                               Attr.hasAttribute(Attribute::Alignment);
-          if (!IsPoisonAttr || Call->isPassingUndefUB(Idx - 1))
-            addAttribute(Attr, Call->getArgOperand(Idx - 1));
----------------
courbet wrote:
> This was broken, right ?
> This was broken, right ?

Yes I believe so. It might 'happen to work' by chance.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105485