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

[Duncan P. N. Exon Smith]

> 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.

>    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.


> +
>    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