[PATCH] D58821: Inline asm constraints: allow ICE-like pointers for the "n" constraint (PR40890)

Eli Friedman via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri Mar 1 12:07:27 PST 2019

efriedma added inline comments.

Comment at: clang/lib/CodeGen/CGStmt.cpp:1852
+        IntResult =
+            llvm::APSInt::get(EVResult.Val.getLValueOffset().getQuantity());
+      else
This always returns an APSInt with width 64; is that really right?  I guess it might not really matter given that it's only going to be used as an immediate constant anyway, but it seems weird.

Comment at: clang/lib/Sema/SemaStmtAsm.cpp:394
+          IntResult = EVResult.Val.getInt();
+        else if (EVResult.Val.isNullPointer())
+          IntResult = llvm::APSInt::get(
APValue::isNullPointer() asserts that the value is an LValue; do you need to check for that explicitly here?

Comment at: clang/lib/Sema/SemaStmtAsm.cpp:399
+          IntResult =
+              llvm::APSInt::get(EVResult.Val.getLValueOffset().getQuantity());
+        else
I think it makes sense to add a method to APValue specifically to do the conversion from LValue to an APSInt, whether or not isNullPointer() is true, and use it both here and in IntExprEvaluator::VisitCastExpr in lib/AST/ExprConstant.cpp.  The logic is sort of subtle (and I'm not completely sure it's right for targets where null is not zero, but you shouldn't try to fix that here).



More information about the cfe-commits mailing list