rjmccall added inline comments.

================
Comment at: include/clang/Basic/TargetInfo.h:959
+  /// \brief Return the target address space which is read only and can be
+  /// casted to the generic address space.
+  virtual llvm::Optional<unsigned> getTargetConstantAddressSpace() const {
----------------
"Return an AST address space which can be used opportunistically for constant 
global memory.  It must be possible to convert pointers into this address space 
to LangAS::Default.  If no such address space exists, this may return None, and 
such optimizations will be disabled."


================
Comment at: lib/CodeGen/CGExpr.cpp:357
+        unsigned AddrSpace =
+            CGF.getTarget().getTargetConstantAddressSpace().getValueOr(0);
         auto *GV = new llvm::GlobalVariable(
----------------
This isn't the right logic.  Part of what I was saying before is that this 
optimization isn't obviously supportable on all targets.  You should call 
getTargetConstantAddressSpace() and then only try to do this optimization if it 
doesn't return None.

For parity, the default implementation should return 0.


================
Comment at: lib/CodeGen/CGExpr.cpp:449
+                         Var, ConvertTypeForMem(E->getType())->getPointerTo()),
                      Object.getAlignment());
     // If the temporary is a global and has a constant initializer or is a
----------------
Every time you're tempted to use getPointerCast, you should instead be using 
the target hook to lift the pointer into the right address space.


https://reviews.llvm.org/D33842



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to