<div dir="ltr">I think the whole point of getting operand bundles fully into the IR is, well, to put them fully into the IR.<div><br></div><div>I like your suggestion of iterators / ranges.</div><div><br></div><div>I would probably sink them as far down into the IR as it makes sense. I'm pretty sure this means CallInst and InvokeInst. I'm not sure whether it would make sense to teach AttributeSet about it -- if you look into it and it makes engineering sense one way or the other, go that way, mail out the patch. =D</div></div><br><div class="gmail_quote"><div dir="ltr">On Thu, Oct 22, 2015 at 4:46 PM Sanjoy Das via llvm-dev <<a href="mailto:llvm-dev@lists.llvm.org">llvm-dev@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Currently I'm trying to "port" over code like this to do the right<br>
thing for calls with operand bundles:<br>
<br>
      CallSite::arg_iterator B = CS.arg_begin(), E = CS.arg_end();<br>
      for (CallSite::arg_iterator A = B; A != E; ++A, ++AI) {<br>
        if (A->get() == V) {<br>
          if (AI == AE) {<br>
            assert(F->isVarArg() &&<br>
                   "More params than args in non-varargs call.");<br>
            return Attribute::None;<br>
          }<br>
          Captures &= !CS.doesNotCapture(A - B);<br>
          if (SCCNodes.count(&*AI))<br>
            continue;<br>
          if (!CS.onlyReadsMemory() && !CS.onlyReadsMemory(A - B))<br>
            return Attribute::None;<br>
          if (!CS.doesNotAccessMemory(A - B))<br>
            IsRead = true;<br>
        }<br>
      }<br>
<br>
The above snippet is from FunctionAttrs.cpp, but there likely are<br>
other similar examples.<br>
<br>
The problem here that the piece of code above assumes that all<br>
interesting inputs to a call / invoke are arguments, and if it<br>
does not find V in the CallSite's argument list then the CallSite does<br>
not read, write or capture it.  This is no longer true with operand<br>
bundles.<br>
<br>
I'm thinking of addressing this by adding an `input_begin()` /<br>
`input_end()` pair of accessors to `CallSite`.  The<br>
`input_begin()` .. `input_end()` range would include all of the<br>
arguments in the `CallSite` *and* all of the operand bundle inputs.<br>
Then I'd generalize accessors on `CallSite` like `doesNotAccessMemory`<br>
and `onlyReadsMemory` to look up either the relevant<br>
`OperandBundleUse` or the `AttributeSet` as appropriate.  Does this<br>
sound reasonable overall?<br>
<br>
A related design point is the depth at which this logic to choose<br>
between the attribute list and operand bundles should live at.  My<br>
intent was to initially keep this as a convenience wrapper in<br>
CallSite, but I can just as easily see this living directly on<br>
`CallInst` and `InvokeInst`.  Perhaps `AttributeSet` needs to directly<br>
support operand bundles?  That may come in handy when later we add<br>
`readonly` `"deopt"` operand bundles.<br>
<br>
Another option is to guard loops like the above with<br>
`!CS.isOperandBundleUse(*U)`.  This is what I tried in<br>
<a href="http://reviews.llvm.org/D13082" rel="noreferrer" target="_blank">http://reviews.llvm.org/D13082</a> and Philip (rightly) pointed out that<br>
having to do this really shows that something is lacking in the<br>
internal APIs.<br>
<br>
Thanks!<br>
-- Sanjoy<br>
_______________________________________________<br>
LLVM Developers mailing list<br>
<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev</a><br>
</blockquote></div>