[PATCH] D122268: Add PointerType analysis for DirectX backend

[Nicolai Hähnle via Phabricator via llvm-commits]

nhaehnle added a comment.

This seems mostly reasonable. I'd point out that if you had an analysis printer, then test cases could be written much more conveniently as lit tests.



================
Comment at: llvm/lib/Target/DirectX/PointerTypeAnalysis.cpp:47
+      else if (PointeeTy != NewPointeeTy)
+        PointeeTy = Type::getInt8Ty(V->getContext());
+    }
----------------
I imagine this should be a TypedPointerType?


================
Comment at: llvm/lib/Target/DirectX/PointerTypeAnalysis.h:1
+//===- Target/DirectX/PointerTypeAnalisis.h - PointerType analysis --------===//
+//
----------------
Typo: Analysis


================
Comment at: llvm/lib/Target/DirectX/PointerTypeAnalysis.h:31
+/// migrated to a custom Type if PointerType loses typed support.
+namespace PointerTypeAnalysis {
+
----------------
Should the namespace be DxilPointerTypeAnalysis since it's target-specific? Also, given that a namespace is used, should the using declaration above be part of the namespace? I don't feel particularly strongly on either point.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D122268/new/

https://reviews.llvm.org/D122268