[llvm-dev] A design question about operand bundles

[Sanjoy Das via llvm-dev]

Currently I'm trying to "port" over code like this to do the right
thing for calls with operand bundles:

      CallSite::arg_iterator B = CS.arg_begin(), E = CS.arg_end();
      for (CallSite::arg_iterator A = B; A != E; ++A, ++AI) {
        if (A->get() == V) {
          if (AI == AE) {
            assert(F->isVarArg() &&
                   "More params than args in non-varargs call.");
            return Attribute::None;
          }
          Captures &= !CS.doesNotCapture(A - B);
          if (SCCNodes.count(&*AI))
            continue;
          if (!CS.onlyReadsMemory() && !CS.onlyReadsMemory(A - B))
            return Attribute::None;
          if (!CS.doesNotAccessMemory(A - B))
            IsRead = true;
        }
      }

The above snippet is from FunctionAttrs.cpp, but there likely are
other similar examples.

The problem here that the piece of code above assumes that all
interesting inputs to a call / invoke are arguments, and if it
does not find V in the CallSite's argument list then the CallSite does
not read, write or capture it.  This is no longer true with operand
bundles.

I'm thinking of addressing this by adding an `input_begin()` /
`input_end()` pair of accessors to `CallSite`.  The
`input_begin()` .. `input_end()` range would include all of the
arguments in the `CallSite` *and* all of the operand bundle inputs.
Then I'd generalize accessors on `CallSite` like `doesNotAccessMemory`
and `onlyReadsMemory` to look up either the relevant
`OperandBundleUse` or the `AttributeSet` as appropriate.  Does this
sound reasonable overall?

A related design point is the depth at which this logic to choose
between the attribute list and operand bundles should live at.  My
intent was to initially keep this as a convenience wrapper in
CallSite, but I can just as easily see this living directly on
`CallInst` and `InvokeInst`.  Perhaps `AttributeSet` needs to directly
support operand bundles?  That may come in handy when later we add
`readonly` `"deopt"` operand bundles.

Another option is to guard loops like the above with
`!CS.isOperandBundleUse(*U)`.  This is what I tried in
http://reviews.llvm.org/D13082 and Philip (rightly) pointed out that
having to do this really shows that something is lacking in the
internal APIs.

Thanks!
-- Sanjoy