[PATCH] [opaque pointer type] Add textual IR support for explicit type parameter to load instruction

[David Blaikie]

On Tue, Feb 24, 2015 at 7:39 PM, Duncan P. N. Exon Smith <
dexonsmith at apple.com> wrote:

> > On 2015-Feb-24, at 15:58, David Blaikie <dblaikie at gmail.com> wrote:
> >
> > ping
> >
> > On Sat, Feb 14, 2015 at 9:39 PM, David Blaikie <dblaikie at gmail.com>
> wrote:
> > Hi rafael, resistor, grosbach, chandlerc, grosser,
> >
> > Essentially the same as the GEP change under review in
> > http://reviews.llvm.org/D7636
> >
> > A similar migration script can be used to update test cases, though a
> few more
> > test case improvements/changes were required this time around:
> (r229269-r229278)
> >
> > import fileinput
> > import sys
> > import re
> >
> > pat = re.compile(r"((?:=|:|^)\s*load (?:atomic )?(?:volatile )?(.*?))(|
> addrspace\(\d+\) *)\*($|
> *(?:%|@|null|undef|blockaddress|getelementptr|addrspacecast|bitcast|inttoptr|\[\[[a-zA-Z]|\{\{).*$)")
> >
> > for line in sys.stdin:
> >   sys.stdout.write(re.sub(pat, r"\1, \2\3*\4", line))
> >
> > http://reviews.llvm.org/D7649
> >
> > Files:
> >   lib/AsmParser/LLParser.cpp
> >   lib/IR/AsmWriter.cpp
> >
> > Index: lib/AsmParser/LLParser.cpp
> > ===================================================================
> > --- lib/AsmParser/LLParser.cpp
> > +++ lib/AsmParser/LLParser.cpp
> > @@ -5165,6 +5165,10 @@
> >      Lex.Lex();
> >    }
> >
> > +  Type *Ty = nullptr;
> > +  if (ParseType(Ty)) return true;
> > +  ParseToken(lltok::comma, "expected comma after getelementptr's type");
> > +
>
> This should be:
>
>     if (ParseType(Ty) ||
>         ParseToken(lltok::comma, "expected comma after load's type"))
>       return true;
>
> (Changed the message and checked the return of ParseToken().)
>
> I think a testcase for this diagnostic is fairly important, since
> it'll be what rejects the old syntax.  I'd suggest something like
> test/Assembler/invalid-load-missing-explicit-type.ll.
>

Diagnostic fixed, ParseToken result checked/returned, test case added.


>
> >    if (ParseTypeAndValue(Val, Loc, PFS) ||
> >        ParseScopeAndOrdering(isAtomic, Scope, Ordering) ||
> >        ParseOptionalCommaAlign(Alignment, AteExtraComma))
> > @@ -5179,6 +5183,9 @@
> >      return Error(Loc, "atomic load cannot use Release ordering");
> >
> >    Inst = new LoadInst(Val, "", isVolatile, Alignment, Ordering, Scope);
> > +
> > +  assert(Ty == Inst->getType());
>
> This should be an `Error()` with a diagnostic, not an `assert()`, since
> this is reachable via:
>
>     %reg = load i32, i8* %mem
>
> (Or isn't it?)
>
> Assuming I'm right, a testcase for this error would be useful as well.
>

Changed assert to error handling and tested.


>
>
> > +
> >    return AteExtraComma ? InstExtraComma : InstNormal;
> >  }
> >
> > Index: lib/IR/AsmWriter.cpp
> > ===================================================================
> > --- lib/IR/AsmWriter.cpp
> > +++ lib/IR/AsmWriter.cpp
> > @@ -2880,6 +2880,11 @@
> >      Out << ", ";
> >      TypePrinter.print(I.getType(), Out);
> >    } else if (Operand) {   // Print the normal way.
> > +    if (auto *LI = dyn_cast<LoadInst>(&I)) {
> > +      Out << ' ';
> > +      TypePrinter.print(LI->getType(), Out);
> > +      Out << ", ";
> > +    }
> >
> >      // PrintAllTypes - Instructions who have operands of all the same
> type
> >      // omit the type from all but the first operand.  If the
> instruction has
> >
> > EMAIL PREFERENCES
> >   http://reviews.llvm.org/settings/panel/emailpreferences/
> >
> > _______________________________________________
> > llvm-commits mailing list
> > llvm-commits at cs.uiuc.edu
> > http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
> >
> >
> > _______________________________________________
> > llvm-commits mailing list
> > llvm-commits at cs.uiuc.edu
> > http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150226/6b71f926/attachment.html>