[clang] Remove Address::deprecated from MveEmitter

[aeubanks]

Summary

We have to keep track of pointer pointee types with opaque pointers.

[aeubanks]

I'm very out of my depth with tablegen, let me know if there's a more elegant way of doing this

[nikic]

clang/utils/TableGen/MveEmitter.cpp

1197

Should be either cast or check for nullptr before dereferencing?

[dmgreen]

I'm very out of my depth with tablegen, let me know if there's a more elegant way of doing this

I think this sounds OK. Providing all the tests pass it looks OK (but I'm not a big expert myself)

[simon_tatham]

Whereas I'm familiar with this code but not with the opaque-pointers effort, so I had to look up the other half of what was going on :-)

If I've understood this correctly, the point is that we should no longer be looking inside a pointer-typed llvm::Value to find out its pointee type, because at some point in the future it will stop storing one at all? And therefore the codegen layer above, if it wants to know the pointee type for a Value it's already made, has to take responsibility for remembering it itself from the point where the Value was constructed.

This seems fine to me, and the uses of address in the actual arm_mve.td don't look as if they're likely to cause trouble with this.

[dblaikie]

clang/utils/TableGen/MveEmitter.cpp

1197

Not sure I follow the question - which variable are you pointing to that needs a null check before its use?

[simon_tatham]

clang/utils/TableGen/MveEmitter.cpp

1197

Oh yes, good point, I see it. dyn_cast<DagInit>(D->getArg(0)) might return nullptr, because the point of dyn_cast is that the thing you're casting might not have the right type. So you should check the return from dyn_cast before you dereference it to call its getOperator method.

(Alternatively, as @nikic hints, if you think that in this case the cast cannot fail, then you can change it to a straight cast which doesn't do the runtime check.)

[aeubanks]

fix dyn_cast

[aeubanks]

clang/utils/TableGen/MveEmitter.cpp

1197

the parentheses were confusing, fixed

[dblaikie]

clang/utils/TableGen/MveEmitter.cpp

1197

oh, right, thanks @simon_tatham - sorry, didn't catch that.

clang/utils/TableGen/MveEmitter.cpp

std::string cName() const override {
 
     if (Const)
       Name = "const " + Name;
     return Name + " *";
   }
   std::string llvmName() const override {
     return "llvm::PointerType::getUnqual(" + Pointee->llvmName() + ")";
   }
+  const Type *getPointeeType() const { return Pointee; }
 
   static bool classof(const Type *T) {
     return T->typeKind() == TypeKind::Pointer;
   }
 };
 
 // Base class for all the types that have a name of the form
 // [prefix][numbers]_t, like int32_t, uint16x8_t, float32x4x2_t.
void morePrerequisites(std::vector<Ptr> &output) const override {
     }
   }
 };
 
 // Result subclass representing making an Address out of a Value.
 class AddressResult : public Result {
 public:
   Ptr Arg;
+  const Type *Ty;
   unsigned Align;
-  AddressResult(Ptr Arg, unsigned Align) : Arg(Arg), Align(Align) {}
+  AddressResult(Ptr Arg, const Type *Ty, unsigned Align)
+      : Arg(Arg), Ty(Ty), Align(Align) {}
   void genCode(raw_ostream &OS,
                CodeGenParamAllocator &ParamAlloc) const override {
-    OS << "Address::deprecated(" << Arg->varname()
+    OS << "Address(" << Arg->varname() << ", " << Ty->llvmName()
        << ", CharUnits::fromQuantity(" << Align << "))";
   }
   std::string typeName() const override {
     return "Address";
   }
   void morePrerequisites(std::vector<Ptr> &output) const override {
     output.push_back(Arg);
   }
if (Op->getName() == "seq") {
     } else if (const auto *PT = dyn_cast<PointerType>(CastType)) {
       return std::make_shared<PointerCastResult>(PT, Arg);
     }
     PrintFatalError("Unsupported type cast");
   } else if (Op->getName() == "address") {
     if (D->getNumArgs() != 2)
       PrintFatalError("'address' should have two arguments");
     Result::Ptr Arg = getCodeForDagArg(D, 0, Scope, Param);
+
+    const Type *Ty = nullptr;
+    if (auto *DI = dyn_cast<DagInit>(D->getArg(0)))
+      if (auto *PTy = dyn_cast<PointerType>(getType(DI->getOperator(), Param)))
+        Ty = PTy->getPointeeType();
+    if (!Ty)
+      PrintFatalError("'address' pointer argument should be a pointer");
+
     unsigned Alignment;
     if (auto *II = dyn_cast<IntInit>(D->getArg(1))) {
       Alignment = II->getValue();
     } else {
       PrintFatalError("'address' alignment argument should be an integer");
     }
-    return std::make_shared<AddressResult>(Arg, Alignment);
+    return std::make_shared<AddressResult>(Arg, Ty, Alignment);
   } else if (Op->getName() == "unsignedflag") {
     if (D->getNumArgs() != 1)
       PrintFatalError("unsignedflag should have exactly one argument");
     Record *TypeRec = cast<DefInit>(D->getArg(0))->getDef();
     if (!TypeRec->isSubClassOf("Type"))
       PrintFatalError("unsignedflag's argument should be a type");
     if (const auto *ST = dyn_cast<ScalarType>(getType(TypeRec, Param))) {
       return std::make_shared<IntLiteralResult>(