[PATCH] D55616: Emit ASM input in a constant context

[Kamil Rytarowski via Phabricator via cfe-commits]

krytarowski added a comment.

@void hi, this broke assembly code on NetBSD for various archs and blocks 
upgrade of the toolchain.

Could you please have a look? https://bugs.llvm.org/show_bug.cgi?id=41027


Repository:
  rC Clang

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

https://reviews.llvm.org/D55616



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

[PATCH] D55616: Emit ASM input in a constant context

[Michał Górny via Phabricator via cfe-commits]

mgorny added a comment.
Herald added a project: clang.

This change apparently introduced a regression: 
https://bugs.llvm.org/show_bug.cgi?id=41027


Repository:
  rC Clang

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

https://reviews.llvm.org/D55616



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

[PATCH] D55616: Emit ASM input in a constant context

[Bill Wendling via Phabricator via cfe-commits]

void added a comment.

> Apologies - the value seems to indeed overflow, but I'm still very confused 
> how this was affected by this change.

What's different is that `setRequiresImmediate` is being set on the constraint 
where before it wasn't. If no range values are supplied (like in this patch), 
it defaults to `INT_MIN` and `INT_MAX`. From GNU's documentation, `n` accepts 
an integer, which we're treating as signed.


Repository:
  rC Clang

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

https://reviews.llvm.org/D55616



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

[PATCH] D55616: Emit ASM input in a constant context

[George Karpenkov via Phabricator via cfe-commits]

george.karpenkov added a comment.

Apologies - the value seems to indeed overflow, but I'm still very confused how 
this was affected by this change.


Repository:
  rC Clang

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

https://reviews.llvm.org/D55616



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

[PATCH] D55616: Emit ASM input in a constant context

[George Karpenkov via Phabricator via cfe-commits]

george.karpenkov added a comment.

@void @efriedma I can't build XNU anymore after this commit, with an error 
message:

  osfmk/i386/genassym.c:298:2: error: value '18446743523953737728' out of range 
for constraint 'n'
  DECLAREULL("KERNEL_BASE", KERNEL_BASE);
  ^~
  osfmk/i386/genassym.c:107:65: note: expanded from macro 'DECLAREULL'
  __asm("DEFINITION__define__" SYM ":\t .ascii \"%0\"" : : "n"  
((unsigned long long)(VAL)))
 
^

The error message looks wrong: the value is in range to fit in `unsigned long 
long` (but I'm not an inline assembly expert).

At a minimum, could we add a test case to this change to show in which cases an 
extra diagnostics would appear?


Repository:
  rC Clang

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

https://reviews.llvm.org/D55616



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

[PATCH] D55616: Emit ASM input in a constant context

[Bill Wendling via Phabricator via cfe-commits]

void added a comment.

In D55616#1335587 , @vitalybuka wrote:

> Looks like it's broken by this patch
>
>   clang: 
> /b/sanitizer-x86_64-linux-bootstrap-msan/build/llvm/tools/clang/lib/AST/ExprConstant.cpp:11055:
>  llvm::APSInt clang::Expr::EvaluateKnownConstInt(const clang::ASTContext &, 
> SmallVectorImpl *) const: Assertion `Result && 
> "Could not evaluate expression"' failed.
>
>
> http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-bootstrap-msan/builds/9281/steps/check-clang%20msan/logs/stdio


This should be fixed now. Sorry about the breakage!


Repository:
  rC Clang

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

https://reviews.llvm.org/D55616



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

[PATCH] D55616: Emit ASM input in a constant context

[Vitaly Buka via Phabricator via cfe-commits]

vitalybuka added a comment.

Looks like it's broken by this patch

  clang: 
/b/sanitizer-x86_64-linux-bootstrap-msan/build/llvm/tools/clang/lib/AST/ExprConstant.cpp:11055:
 llvm::APSInt clang::Expr::EvaluateKnownConstInt(const clang::ASTContext &, 
SmallVectorImpl *) const: Assertion `Result && 
"Could not evaluate expression"' failed.

http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-bootstrap-msan/builds/9281/steps/check-clang%20msan/logs/stdio


Repository:
  rC Clang

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

https://reviews.llvm.org/D55616



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

[PATCH] D55616: Emit ASM input in a constant context

[Bill Wendling via Phabricator via cfe-commits]

void updated this revision to Diff 178793.
void added a comment.

Fix how prefixes are used in the testcase.


Repository:
  rC Clang

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

https://reviews.llvm.org/D55616

Files:
  lib/Basic/TargetInfo.cpp
  lib/CodeGen/CGStmt.cpp
  lib/Sema/SemaStmtAsm.cpp
  test/CodeGen/builtin-constant-p.c


Index: test/CodeGen/builtin-constant-p.c
===
--- test/CodeGen/builtin-constant-p.c
+++ test/CodeGen/builtin-constant-p.c
@@ -1,4 +1,5 @@
 // RUN: %clang_cc1 -triple x86_64-unknown-unknown -emit-llvm -o - %s -O2 | 
FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -emit-llvm -o - %s -O0 | 
FileCheck --check-prefix=O0 %s
 
 int a = 42;
 
@@ -166,3 +167,13 @@
 
 extern char test16_v;
 struct { int a; } test16 = { __builtin_constant_p(test16_v) };
+
+extern unsigned long long test17_v;
+
+void test17() {
+  // O0: define void @test17
+  // O0: call void asm sideeffect "", {{.*}}(i32 -1) 
+  // CHECK: define void @test17
+  // CHECK: call void asm sideeffect "", {{.*}}(i32 -1) 
+  __asm__ __volatile__("" :: "n"( (__builtin_constant_p(test17_v) || 0) ? 1 : 
-1));
+}
Index: lib/Sema/SemaStmtAsm.cpp
===
--- lib/Sema/SemaStmtAsm.cpp
+++ lib/Sema/SemaStmtAsm.cpp
@@ -378,17 +378,17 @@
  << InputExpr->getSourceRange());
 } else if (Info.requiresImmediateConstant() && !Info.allowsRegister()) {
   if (!InputExpr->isValueDependent()) {
-Expr::EvalResult EVResult;
-if (!InputExpr->EvaluateAsInt(EVResult, Context))
+llvm::SmallVector Diags;
+llvm::APSInt Result = InputExpr->EvaluateKnownConstInt(Context, 
);
+if (!Diags.empty())
   return StmtError(
   Diag(InputExpr->getBeginLoc(), diag::err_asm_immediate_expected)
   << Info.getConstraintStr() << InputExpr->getSourceRange());
-llvm::APSInt Result = EVResult.Val.getInt();
- if (!Info.isValidAsmImmediate(Result))
-   return StmtError(Diag(InputExpr->getBeginLoc(),
- diag::err_invalid_asm_value_for_constraint)
-<< Result.toString(10) << Info.getConstraintStr()
-<< InputExpr->getSourceRange());
+if (!Info.isValidAsmImmediate(Result))
+  return StmtError(Diag(InputExpr->getBeginLoc(),
+diag::err_invalid_asm_value_for_constraint)
+   << Result.toString(10) << Info.getConstraintStr()
+   << InputExpr->getSourceRange());
   }
 
 } else {
Index: lib/CodeGen/CGStmt.cpp
===
--- lib/CodeGen/CGStmt.cpp
+++ lib/CodeGen/CGStmt.cpp
@@ -1820,11 +1820,14 @@
   // If this can't be a register or memory, i.e., has to be a constant
   // (immediate or symbolic), try to emit it as such.
   if (!Info.allowsRegister() && !Info.allowsMemory()) {
+if (Info.requiresImmediateConstant()) {
+  llvm::APSInt AsmConst = InputExpr->EvaluateKnownConstInt(getContext());
+  return llvm::ConstantInt::get(getLLVMContext(), AsmConst);
+}
+
 Expr::EvalResult Result;
 if (InputExpr->EvaluateAsInt(Result, getContext()))
   return llvm::ConstantInt::get(getLLVMContext(), Result.Val.getInt());
-assert(!Info.requiresImmediateConstant() &&
-   "Required-immediate inlineasm arg isn't constant?");
   }
 
   if (Info.allowsRegister() || !Info.allowsMemory())
Index: lib/Basic/TargetInfo.cpp
===
--- lib/Basic/TargetInfo.cpp
+++ lib/Basic/TargetInfo.cpp
@@ -685,7 +685,9 @@
   // FIXME: Fail if % is used with the last operand.
   break;
 case 'i': // immediate integer.
+  break;
 case 'n': // immediate integer with a known value.
+  Info.setRequiresImmediate();
   break;
 case 'I':  // Various constant constraints with target-specific meanings.
 case 'J':


Index: test/CodeGen/builtin-constant-p.c
===
--- test/CodeGen/builtin-constant-p.c
+++ test/CodeGen/builtin-constant-p.c
@@ -1,4 +1,5 @@
 // RUN: %clang_cc1 -triple x86_64-unknown-unknown -emit-llvm -o - %s -O2 | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -emit-llvm -o - %s -O0 | FileCheck --check-prefix=O0 %s
 
 int a = 42;
 
@@ -166,3 +167,13 @@
 
 extern char test16_v;
 struct { int a; } test16 = { __builtin_constant_p(test16_v) };
+
+extern unsigned long long test17_v;
+
+void test17() {
+  // O0: define void @test17
+  // O0: call void asm sideeffect "", {{.*}}(i32 -1) 
+  // CHECK: define void @test17
+  // CHECK: call void asm sideeffect "", {{.*}}(i32 -1) 
+  __asm__ __volatile__("" :: "n"( (__builtin_constant_p(test17_v) || 0) ? 1 : -1));
+}
Index: lib/Sema/SemaStmtAsm.cpp

[PATCH] D55616: Emit ASM input in a constant context

[Bill Wendling via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC349561: Emit ASM input in a constant context (authored by 
void, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D55616?vs=178793=178794#toc

Repository:
  rC Clang

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

https://reviews.llvm.org/D55616

Files:
  lib/Basic/TargetInfo.cpp
  lib/CodeGen/CGStmt.cpp
  lib/Sema/SemaStmtAsm.cpp
  test/CodeGen/builtin-constant-p.c


Index: lib/CodeGen/CGStmt.cpp
===
--- lib/CodeGen/CGStmt.cpp
+++ lib/CodeGen/CGStmt.cpp
@@ -1820,11 +1820,14 @@
   // If this can't be a register or memory, i.e., has to be a constant
   // (immediate or symbolic), try to emit it as such.
   if (!Info.allowsRegister() && !Info.allowsMemory()) {
+if (Info.requiresImmediateConstant()) {
+  llvm::APSInt AsmConst = InputExpr->EvaluateKnownConstInt(getContext());
+  return llvm::ConstantInt::get(getLLVMContext(), AsmConst);
+}
+
 Expr::EvalResult Result;
 if (InputExpr->EvaluateAsInt(Result, getContext()))
   return llvm::ConstantInt::get(getLLVMContext(), Result.Val.getInt());
-assert(!Info.requiresImmediateConstant() &&
-   "Required-immediate inlineasm arg isn't constant?");
   }
 
   if (Info.allowsRegister() || !Info.allowsMemory())
Index: lib/Sema/SemaStmtAsm.cpp
===
--- lib/Sema/SemaStmtAsm.cpp
+++ lib/Sema/SemaStmtAsm.cpp
@@ -378,17 +378,17 @@
  << InputExpr->getSourceRange());
 } else if (Info.requiresImmediateConstant() && !Info.allowsRegister()) {
   if (!InputExpr->isValueDependent()) {
-Expr::EvalResult EVResult;
-if (!InputExpr->EvaluateAsInt(EVResult, Context))
+llvm::SmallVector Diags;
+llvm::APSInt Result = InputExpr->EvaluateKnownConstInt(Context, 
);
+if (!Diags.empty())
   return StmtError(
   Diag(InputExpr->getBeginLoc(), diag::err_asm_immediate_expected)
   << Info.getConstraintStr() << InputExpr->getSourceRange());
-llvm::APSInt Result = EVResult.Val.getInt();
- if (!Info.isValidAsmImmediate(Result))
-   return StmtError(Diag(InputExpr->getBeginLoc(),
- diag::err_invalid_asm_value_for_constraint)
-<< Result.toString(10) << Info.getConstraintStr()
-<< InputExpr->getSourceRange());
+if (!Info.isValidAsmImmediate(Result))
+  return StmtError(Diag(InputExpr->getBeginLoc(),
+diag::err_invalid_asm_value_for_constraint)
+   << Result.toString(10) << Info.getConstraintStr()
+   << InputExpr->getSourceRange());
   }
 
 } else {
Index: lib/Basic/TargetInfo.cpp
===
--- lib/Basic/TargetInfo.cpp
+++ lib/Basic/TargetInfo.cpp
@@ -685,7 +685,9 @@
   // FIXME: Fail if % is used with the last operand.
   break;
 case 'i': // immediate integer.
+  break;
 case 'n': // immediate integer with a known value.
+  Info.setRequiresImmediate();
   break;
 case 'I':  // Various constant constraints with target-specific meanings.
 case 'J':
Index: test/CodeGen/builtin-constant-p.c
===
--- test/CodeGen/builtin-constant-p.c
+++ test/CodeGen/builtin-constant-p.c
@@ -1,4 +1,5 @@
 // RUN: %clang_cc1 -triple x86_64-unknown-unknown -emit-llvm -o - %s -O2 | 
FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -emit-llvm -o - %s -O0 | 
FileCheck --check-prefix=O0 %s
 
 int a = 42;
 
@@ -166,3 +167,13 @@
 
 extern char test16_v;
 struct { int a; } test16 = { __builtin_constant_p(test16_v) };
+
+extern unsigned long long test17_v;
+
+void test17() {
+  // O0: define void @test17
+  // O0: call void asm sideeffect "", {{.*}}(i32 -1) 
+  // CHECK: define void @test17
+  // CHECK: call void asm sideeffect "", {{.*}}(i32 -1) 
+  __asm__ __volatile__("" :: "n"( (__builtin_constant_p(test17_v) || 0) ? 1 : 
-1));
+}


Index: lib/CodeGen/CGStmt.cpp
===
--- lib/CodeGen/CGStmt.cpp
+++ lib/CodeGen/CGStmt.cpp
@@ -1820,11 +1820,14 @@
   // If this can't be a register or memory, i.e., has to be a constant
   // (immediate or symbolic), try to emit it as such.
   if (!Info.allowsRegister() && !Info.allowsMemory()) {
+if (Info.requiresImmediateConstant()) {
+  llvm::APSInt AsmConst = InputExpr->EvaluateKnownConstInt(getContext());
+  return llvm::ConstantInt::get(getLLVMContext(), AsmConst);
+}
+
 Expr::EvalResult Result;
 if (InputExpr->EvaluateAsInt(Result, getContext()))
   return llvm::ConstantInt::get(getLLVMContext(), Result.Val.getInt());
-

[PATCH] D55616: Emit ASM input in a constant context

[Bill Wendling via Phabricator via cfe-commits]

void marked an inline comment as done.
void added inline comments.



Comment at: test/CodeGen/builtin-constant-p.c:2
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -emit-llvm -o - %s -O2 | 
FileCheck --check-prefix=O2 %s
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -emit-llvm -o - %s -O0 | 
FileCheck --check-prefix=O0 %s
 

efriedma wrote:
> efriedma wrote:
> > void wrote:
> > > efriedma wrote:
> > > > Given this code doesn't check the optimization level anymore, do you 
> > > > still need separate check prefixes for `O2` and `O0`.  Or if that 
> > > > doesn't work for everything, maybe you could share a check prefix for 
> > > > some of the tests? (Maybe it would make sense to check IR generated 
> > > > using `-disable-llvm-passes`.)
> > > The bug only triggered at `O0`, so I still want to test it without 
> > > optimizations. Note that we do check optimization levels during code 
> > > generation to determine if we should generate an `is.constant` intrinsic.
> > You can use something like "-check-prefixes=CHECK,O0" to reduce the 
> > duplication.
> That doesn't pass.
> 
> More specifically, I was thinking something more along the lines of using 
> "--check-prefixes=CHECK,O2" for the first run, and 
> "--check-prefixes=CHECK,O0" for the second run; then you can use "CHECK:" for 
> the common lines and "O2:"/"O0:" for the lines that are different.
I think think that works. The thing is that there should rarely be common lines 
between `O2` and `O0`. I understand what you're going for here, but I don't 
think it's beneficial to this testcase. However, I don't need to change all of 
the `CHECK`s either. I came up with a compromise...


Repository:
  rC Clang

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

https://reviews.llvm.org/D55616



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

[PATCH] D55616: Emit ASM input in a constant context

[Eli Friedman via Phabricator via cfe-commits]

efriedma added inline comments.



Comment at: test/CodeGen/builtin-constant-p.c:2
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -emit-llvm -o - %s -O2 | 
FileCheck --check-prefix=O2 %s
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -emit-llvm -o - %s -O0 | 
FileCheck --check-prefix=O0 %s
 

efriedma wrote:
> void wrote:
> > efriedma wrote:
> > > Given this code doesn't check the optimization level anymore, do you 
> > > still need separate check prefixes for `O2` and `O0`.  Or if that doesn't 
> > > work for everything, maybe you could share a check prefix for some of the 
> > > tests? (Maybe it would make sense to check IR generated using 
> > > `-disable-llvm-passes`.)
> > The bug only triggered at `O0`, so I still want to test it without 
> > optimizations. Note that we do check optimization levels during code 
> > generation to determine if we should generate an `is.constant` intrinsic.
> You can use something like "-check-prefixes=CHECK,O0" to reduce the 
> duplication.
That doesn't pass.

More specifically, I was thinking something more along the lines of using 
"--check-prefixes=CHECK,O2" for the first run, and "--check-prefixes=CHECK,O0" 
for the second run; then you can use "CHECK:" for the common lines and 
"O2:"/"O0:" for the lines that are different.


Repository:
  rC Clang

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

https://reviews.llvm.org/D55616



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

[PATCH] D55616: Emit ASM input in a constant context

[Bill Wendling via Phabricator via cfe-commits]

void updated this revision to Diff 178582.
void added a comment.

Reduce duplication by using multiple check prefixes.


Repository:
  rC Clang

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

https://reviews.llvm.org/D55616

Files:
  lib/Basic/TargetInfo.cpp
  lib/CodeGen/CGStmt.cpp
  lib/Sema/SemaStmtAsm.cpp
  test/CodeGen/builtin-constant-p.c


Index: test/CodeGen/builtin-constant-p.c
===
--- test/CodeGen/builtin-constant-p.c
+++ test/CodeGen/builtin-constant-p.c
@@ -1,4 +1,5 @@
 // RUN: %clang_cc1 -triple x86_64-unknown-unknown -emit-llvm -o - %s -O2 | 
FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -emit-llvm -o - %s -O0 | 
FileCheck --check-prefixes=CHECK,O0 %s
 
 int a = 42;
 
@@ -166,3 +167,13 @@
 
 extern char test16_v;
 struct { int a; } test16 = { __builtin_constant_p(test16_v) };
+
+extern unsigned long long test17_v;
+
+void test17() {
+  // O0: define void @test17
+  // O0: call void asm sideeffect "", {{.*}}(i32 -1) 
+  // CHECK: define void @test17
+  // CHECK: call void asm sideeffect "", {{.*}}(i32 -1) 
+  __asm__ __volatile__("" :: "n"( (__builtin_constant_p(test17_v) || 0) ? 1 : 
-1));
+}
Index: lib/Sema/SemaStmtAsm.cpp
===
--- lib/Sema/SemaStmtAsm.cpp
+++ lib/Sema/SemaStmtAsm.cpp
@@ -378,17 +378,17 @@
  << InputExpr->getSourceRange());
 } else if (Info.requiresImmediateConstant() && !Info.allowsRegister()) {
   if (!InputExpr->isValueDependent()) {
-Expr::EvalResult EVResult;
-if (!InputExpr->EvaluateAsInt(EVResult, Context))
+llvm::SmallVector Diags;
+llvm::APSInt Result = InputExpr->EvaluateKnownConstInt(Context, 
);
+if (!Diags.empty())
   return StmtError(
   Diag(InputExpr->getBeginLoc(), diag::err_asm_immediate_expected)
   << Info.getConstraintStr() << InputExpr->getSourceRange());
-llvm::APSInt Result = EVResult.Val.getInt();
- if (!Info.isValidAsmImmediate(Result))
-   return StmtError(Diag(InputExpr->getBeginLoc(),
- diag::err_invalid_asm_value_for_constraint)
-<< Result.toString(10) << Info.getConstraintStr()
-<< InputExpr->getSourceRange());
+if (!Info.isValidAsmImmediate(Result))
+  return StmtError(Diag(InputExpr->getBeginLoc(),
+diag::err_invalid_asm_value_for_constraint)
+   << Result.toString(10) << Info.getConstraintStr()
+   << InputExpr->getSourceRange());
   }
 
 } else {
Index: lib/CodeGen/CGStmt.cpp
===
--- lib/CodeGen/CGStmt.cpp
+++ lib/CodeGen/CGStmt.cpp
@@ -1820,11 +1820,14 @@
   // If this can't be a register or memory, i.e., has to be a constant
   // (immediate or symbolic), try to emit it as such.
   if (!Info.allowsRegister() && !Info.allowsMemory()) {
+if (Info.requiresImmediateConstant()) {
+  llvm::APSInt AsmConst = InputExpr->EvaluateKnownConstInt(getContext());
+  return llvm::ConstantInt::get(getLLVMContext(), AsmConst);
+}
+
 Expr::EvalResult Result;
 if (InputExpr->EvaluateAsInt(Result, getContext()))
   return llvm::ConstantInt::get(getLLVMContext(), Result.Val.getInt());
-assert(!Info.requiresImmediateConstant() &&
-   "Required-immediate inlineasm arg isn't constant?");
   }
 
   if (Info.allowsRegister() || !Info.allowsMemory())
Index: lib/Basic/TargetInfo.cpp
===
--- lib/Basic/TargetInfo.cpp
+++ lib/Basic/TargetInfo.cpp
@@ -685,7 +685,9 @@
   // FIXME: Fail if % is used with the last operand.
   break;
 case 'i': // immediate integer.
+  break;
 case 'n': // immediate integer with a known value.
+  Info.setRequiresImmediate();
   break;
 case 'I':  // Various constant constraints with target-specific meanings.
 case 'J':


Index: test/CodeGen/builtin-constant-p.c
===
--- test/CodeGen/builtin-constant-p.c
+++ test/CodeGen/builtin-constant-p.c
@@ -1,4 +1,5 @@
 // RUN: %clang_cc1 -triple x86_64-unknown-unknown -emit-llvm -o - %s -O2 | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -emit-llvm -o - %s -O0 | FileCheck --check-prefixes=CHECK,O0 %s
 
 int a = 42;
 
@@ -166,3 +167,13 @@
 
 extern char test16_v;
 struct { int a; } test16 = { __builtin_constant_p(test16_v) };
+
+extern unsigned long long test17_v;
+
+void test17() {
+  // O0: define void @test17
+  // O0: call void asm sideeffect "", {{.*}}(i32 -1) 
+  // CHECK: define void @test17
+  // CHECK: call void asm sideeffect "", {{.*}}(i32 -1) 
+  __asm__ __volatile__("" :: "n"( (__builtin_constant_p(test17_v) || 0) ? 1 : -1));
+}
Index: 

[PATCH] D55616: Emit ASM input in a constant context

[Eli Friedman via Phabricator via cfe-commits]

efriedma accepted this revision.
efriedma added a comment.
This revision is now accepted and ready to land.

LGTM with a minor nit, but give a couple days for @jyknight to add any more 
comments.




Comment at: test/CodeGen/builtin-constant-p.c:2
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -emit-llvm -o - %s -O2 | 
FileCheck --check-prefix=O2 %s
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -emit-llvm -o - %s -O0 | 
FileCheck --check-prefix=O0 %s
 

void wrote:
> efriedma wrote:
> > Given this code doesn't check the optimization level anymore, do you still 
> > need separate check prefixes for `O2` and `O0`.  Or if that doesn't work 
> > for everything, maybe you could share a check prefix for some of the tests? 
> > (Maybe it would make sense to check IR generated using 
> > `-disable-llvm-passes`.)
> The bug only triggered at `O0`, so I still want to test it without 
> optimizations. Note that we do check optimization levels during code 
> generation to determine if we should generate an `is.constant` intrinsic.
You can use something like "-check-prefixes=CHECK,O0" to reduce the duplication.


Repository:
  rC Clang

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

https://reviews.llvm.org/D55616



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

[PATCH] D55616: Emit ASM input in a constant context

[Bill Wendling via Phabricator via cfe-commits]

void marked an inline comment as done.
void added inline comments.



Comment at: test/CodeGen/builtin-constant-p.c:2
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -emit-llvm -o - %s -O2 | 
FileCheck --check-prefix=O2 %s
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -emit-llvm -o - %s -O0 | 
FileCheck --check-prefix=O0 %s
 

efriedma wrote:
> Given this code doesn't check the optimization level anymore, do you still 
> need separate check prefixes for `O2` and `O0`.  Or if that doesn't work for 
> everything, maybe you could share a check prefix for some of the tests? 
> (Maybe it would make sense to check IR generated using 
> `-disable-llvm-passes`.)
The bug only triggered at `O0`, so I still want to test it without 
optimizations. Note that we do check optimization levels during code generation 
to determine if we should generate an `is.constant` intrinsic.


Repository:
  rC Clang

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

https://reviews.llvm.org/D55616



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

[PATCH] D55616: Emit ASM input in a constant context

[Eli Friedman via Phabricator via cfe-commits]

efriedma added inline comments.



Comment at: test/CodeGen/builtin-constant-p.c:2
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -emit-llvm -o - %s -O2 | 
FileCheck --check-prefix=O2 %s
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -emit-llvm -o - %s -O0 | 
FileCheck --check-prefix=O0 %s
 

Given this code doesn't check the optimization level anymore, do you still need 
separate check prefixes for `O2` and `O0`.  Or if that doesn't work for 
everything, maybe you could share a check prefix for some of the tests? (Maybe 
it would make sense to check IR generated using `-disable-llvm-passes`.)


Repository:
  rC Clang

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

https://reviews.llvm.org/D55616



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

[PATCH] D55616: Emit ASM input in a constant context

[Bill Wendling via Phabricator via cfe-commits]

void added a comment.

Ping? :-)


Repository:
  rC Clang

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

https://reviews.llvm.org/D55616



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

[PATCH] D55616: Emit ASM input in a constant context

[Bill Wendling via Phabricator via cfe-commits]

void added a comment.

In D55616#1329301 , @jyknight wrote:

> This seems like a good start, but not complete.
>
> "n" and "i" both should require that their argument is a constant expression. 
> For "n", it actually must be an immediate constant integer, so 
> setRequiresImmediate() should be used there. For "i", you may use an lvalue 
> constant as well. The way we seem to indicate that, today, is with `if 
> (!Info.allowsRegister() && !Info.allowsMemory())`. However the code in that 
> block does not today *require* that the evaluation as a constant 
> succeed...and it should.
>
> It should also only require that the result is an integer when 
> `requiresImmediateConstant()`.
>
> Additionally, the Sema code needs to have the same conditions as the CodeGen 
> code for when a constant expression is required, but it doesn't. (before or 
> after your change).


Have I mentioned how much I *love* inline asm? :-)

I think I've addressed all of your concerns. PTAL.


Repository:
  rC Clang

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

https://reviews.llvm.org/D55616



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

[PATCH] D55616: Emit ASM input in a constant context

[Bill Wendling via Phabricator via cfe-commits]

void updated this revision to Diff 178158.
void added a comment.

The `n` constriant *requires* a known integer. Therefore, enforce this in both
Sema and CodeGen by setting the "requires immediate" flag and evaluating to a
known integer instead of random "int"..

This doesn't so much address `i`, which has other semantics. However, it
shouldn't regress how the `i` constraint is currently used.


Repository:
  rC Clang

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

https://reviews.llvm.org/D55616

Files:
  lib/Basic/TargetInfo.cpp
  lib/CodeGen/CGStmt.cpp
  lib/Sema/SemaStmtAsm.cpp
  test/CodeGen/builtin-constant-p.c

Index: test/CodeGen/builtin-constant-p.c
===
--- test/CodeGen/builtin-constant-p.c
+++ test/CodeGen/builtin-constant-p.c
@@ -1,4 +1,5 @@
-// RUN: %clang_cc1 -triple x86_64-unknown-unknown -emit-llvm -o - %s -O2 | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -emit-llvm -o - %s -O2 | FileCheck --check-prefix=O2 %s
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -emit-llvm -o - %s -O0 | FileCheck --check-prefix=O0 %s
 
 int a = 42;
 
@@ -14,8 +15,8 @@
 struct foo f = (struct foo){ __builtin_constant_p(y), 42 };
 
 struct foo test0(int expr) {
-  // CHECK: define i64 @test0(i32 %expr)
-  // CHECK: call i1 @llvm.is.constant.i32(i32 %expr)
+  // O2: define i64 @test0(i32 %expr)
+  // O2: call i1 @llvm.is.constant.i32(i32 %expr)
   struct foo f = (struct foo){ __builtin_constant_p(expr), 42 };
   return f;
 }
@@ -27,15 +28,15 @@
 }
 
 int test1() {
-  // CHECK: define i32 @test1
-  // CHECK: add nsw i32 %0, -13
-  // CHECK-NEXT: call i1 @llvm.is.constant.i32(i32 %sub)
+  // O2: define i32 @test1
+  // O2: add nsw i32 %0, -13
+  // O2-NEXT: call i1 @llvm.is.constant.i32(i32 %sub)
   return bcp(test1_i() - 13);
 }
 
 int test2() {
-  // CHECK: define i32 @test2
-  // CHECK: ret i32 0
+  // O2: define i32 @test2
+  // O2: ret i32 0
   return __builtin_constant_p( - 13);
 }
 
@@ -44,8 +45,8 @@
 }
 
 int test3() {
-  // CHECK: define i32 @test3
-  // CHECK: ret i32 1
+  // O2: define i32 @test3
+  // O2: ret i32 1
   return bcp(test3_i() - 13);
 }
 
@@ -54,16 +55,16 @@
 int b[] = {1, 2, 3};
 
 int test4() {
-  // CHECK: define i32 @test4
-  // CHECK: ret i32 0
+  // O2: define i32 @test4
+  // O2: ret i32 0
   return __builtin_constant_p(b);
 }
 
 const char test5_c[] = {1, 2, 3, 0};
 
 int test5() {
-  // CHECK: define i32 @test5
-  // CHECK: ret i32 0
+  // O2: define i32 @test5
+  // O2: ret i32 0
   return __builtin_constant_p(test5_c);
 }
 
@@ -72,16 +73,16 @@
 }
 
 int test6() {
-  // CHECK: define i32 @test6
-  // CHECK: ret i32 0
+  // O2: define i32 @test6
+  // O2: ret i32 0
   return __builtin_constant_p(test6_i(test5_c));
 }
 
 /* --- Non-constant global variables */
 
 int test7() {
-  // CHECK: define i32 @test7
-  // CHECK: call i1 @llvm.is.constant.i32(i32 %0)
+  // O2: define i32 @test7
+  // O2: call i1 @llvm.is.constant.i32(i32 %0)
   return bcp(a);
 }
 
@@ -90,8 +91,8 @@
 const int c = 42;
 
 int test8() {
-  // CHECK: define i32 @test8
-  // CHECK: ret i32 1
+  // O2: define i32 @test8
+  // O2: ret i32 1
   return bcp(c);
 }
 
@@ -101,34 +102,34 @@
 const int c_arr[] = { 1, 2, 3 };
 
 int test9() {
-  // CHECK: define i32 @test9
-  // CHECK: call i1 @llvm.is.constant.i32(i32 %0)
+  // O2: define i32 @test9
+  // O2: call i1 @llvm.is.constant.i32(i32 %0)
   return __builtin_constant_p(arr[2]);
 }
 
 int test10() {
-  // CHECK: define i32 @test10
-  // CHECK: ret i32 1
+  // O2: define i32 @test10
+  // O2: ret i32 1
   return __builtin_constant_p(c_arr[2]);
 }
 
 int test11() {
-  // CHECK: define i32 @test11
-  // CHECK: ret i32 0
+  // O2: define i32 @test11
+  // O2: ret i32 0
   return __builtin_constant_p(c_arr);
 }
 
 /* --- Function pointers */
 
 int test12() {
-  // CHECK: define i32 @test12
-  // CHECK: ret i32 0
+  // O2: define i32 @test12
+  // O2: ret i32 0
   return __builtin_constant_p();
 }
 
 int test13() {
-  // CHECK: define i32 @test13
-  // CHECK: ret i32 1
+  // O2: define i32 @test13
+  // O2: ret i32 1
   return __builtin_constant_p( != 0);
 }
 
@@ -166,3 +167,13 @@
 
 extern char test16_v;
 struct { int a; } test16 = { __builtin_constant_p(test16_v) };
+
+extern unsigned long long test17_v;
+
+void test17() {
+  // O0: define void @test17
+  // O0: call void asm sideeffect "", {{.*}}(i32 -1) 
+  // O2: define void @test17
+  // O2: call void asm sideeffect "", {{.*}}(i32 -1) 
+  __asm__ __volatile__("" :: "n"( (__builtin_constant_p(test17_v) || 0) ? 1 : -1));
+}
Index: lib/Sema/SemaStmtAsm.cpp
===
--- lib/Sema/SemaStmtAsm.cpp
+++ lib/Sema/SemaStmtAsm.cpp
@@ -378,17 +378,17 @@
  << InputExpr->getSourceRange());
 } else if (Info.requiresImmediateConstant() && !Info.allowsRegister()) {
   if (!InputExpr->isValueDependent()) {
-Expr::EvalResult EVResult;
- 

[PATCH] D55616: Emit ASM input in a constant context

[James Y Knight via Phabricator via cfe-commits]

jyknight added a comment.

This seems like a good start, but not complete.

"n" and "i" both should require that their argument is a constant expression. 
For "n", it actually must be an immediate constant integer, so 
setRequiresImmediate() should be used there. For "i", you may use an lvalue 
constant as well. The way we seem to indicate that, today, is with `if 
(!Info.allowsRegister() && !Info.allowsMemory())`. However the code in that 
block does not today *require* that the evaluation as a constant succeed...and 
it should.

It should also only require that the result is an integer when 
`requiresImmediateConstant()`.

Additionally, the Sema code needs to have the same conditions as the CodeGen 
code for when a constant expression is required, but it doesn't. (before or 
after your change).


Repository:
  rC Clang

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

https://reviews.llvm.org/D55616



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

[PATCH] D55616: Emit ASM input in a constant context

[Bill Wendling via Phabricator via cfe-commits]

void added a comment.

Addressed Eli's comment.


Repository:
  rC Clang

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

https://reviews.llvm.org/D55616



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

[PATCH] D55616: Emit ASM input in a constant context

[Bill Wendling via Phabricator via cfe-commits]

void updated this revision to Diff 177936.
void marked an inline comment as done.
void added a comment.

Don't look at the optimization level here.


Repository:
  rC Clang

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

https://reviews.llvm.org/D55616

Files:
  include/clang/AST/Expr.h
  lib/AST/ExprConstant.cpp
  lib/CodeGen/CGStmt.cpp
  test/CodeGen/builtin-constant-p.c

Index: test/CodeGen/builtin-constant-p.c
===
--- test/CodeGen/builtin-constant-p.c
+++ test/CodeGen/builtin-constant-p.c
@@ -1,4 +1,5 @@
-// RUN: %clang_cc1 -triple x86_64-unknown-unknown -emit-llvm -o - %s -O2 | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -emit-llvm -o - %s -O2 | FileCheck --check-prefix=O2 %s
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -emit-llvm -o - %s -O0 | FileCheck --check-prefix=O0 %s
 
 int a = 42;
 
@@ -14,8 +15,8 @@
 struct foo f = (struct foo){ __builtin_constant_p(y), 42 };
 
 struct foo test0(int expr) {
-  // CHECK: define i64 @test0(i32 %expr)
-  // CHECK: call i1 @llvm.is.constant.i32(i32 %expr)
+  // O2: define i64 @test0(i32 %expr)
+  // O2: call i1 @llvm.is.constant.i32(i32 %expr)
   struct foo f = (struct foo){ __builtin_constant_p(expr), 42 };
   return f;
 }
@@ -27,15 +28,15 @@
 }
 
 int test1() {
-  // CHECK: define i32 @test1
-  // CHECK: add nsw i32 %0, -13
-  // CHECK-NEXT: call i1 @llvm.is.constant.i32(i32 %sub)
+  // O2: define i32 @test1
+  // O2: add nsw i32 %0, -13
+  // O2-NEXT: call i1 @llvm.is.constant.i32(i32 %sub)
   return bcp(test1_i() - 13);
 }
 
 int test2() {
-  // CHECK: define i32 @test2
-  // CHECK: ret i32 0
+  // O2: define i32 @test2
+  // O2: ret i32 0
   return __builtin_constant_p( - 13);
 }
 
@@ -44,8 +45,8 @@
 }
 
 int test3() {
-  // CHECK: define i32 @test3
-  // CHECK: ret i32 1
+  // O2: define i32 @test3
+  // O2: ret i32 1
   return bcp(test3_i() - 13);
 }
 
@@ -54,16 +55,16 @@
 int b[] = {1, 2, 3};
 
 int test4() {
-  // CHECK: define i32 @test4
-  // CHECK: ret i32 0
+  // O2: define i32 @test4
+  // O2: ret i32 0
   return __builtin_constant_p(b);
 }
 
 const char test5_c[] = {1, 2, 3, 0};
 
 int test5() {
-  // CHECK: define i32 @test5
-  // CHECK: ret i32 0
+  // O2: define i32 @test5
+  // O2: ret i32 0
   return __builtin_constant_p(test5_c);
 }
 
@@ -72,16 +73,16 @@
 }
 
 int test6() {
-  // CHECK: define i32 @test6
-  // CHECK: ret i32 0
+  // O2: define i32 @test6
+  // O2: ret i32 0
   return __builtin_constant_p(test6_i(test5_c));
 }
 
 /* --- Non-constant global variables */
 
 int test7() {
-  // CHECK: define i32 @test7
-  // CHECK: call i1 @llvm.is.constant.i32(i32 %0)
+  // O2: define i32 @test7
+  // O2: call i1 @llvm.is.constant.i32(i32 %0)
   return bcp(a);
 }
 
@@ -90,8 +91,8 @@
 const int c = 42;
 
 int test8() {
-  // CHECK: define i32 @test8
-  // CHECK: ret i32 1
+  // O2: define i32 @test8
+  // O2: ret i32 1
   return bcp(c);
 }
 
@@ -101,34 +102,34 @@
 const int c_arr[] = { 1, 2, 3 };
 
 int test9() {
-  // CHECK: define i32 @test9
-  // CHECK: call i1 @llvm.is.constant.i32(i32 %0)
+  // O2: define i32 @test9
+  // O2: call i1 @llvm.is.constant.i32(i32 %0)
   return __builtin_constant_p(arr[2]);
 }
 
 int test10() {
-  // CHECK: define i32 @test10
-  // CHECK: ret i32 1
+  // O2: define i32 @test10
+  // O2: ret i32 1
   return __builtin_constant_p(c_arr[2]);
 }
 
 int test11() {
-  // CHECK: define i32 @test11
-  // CHECK: ret i32 0
+  // O2: define i32 @test11
+  // O2: ret i32 0
   return __builtin_constant_p(c_arr);
 }
 
 /* --- Function pointers */
 
 int test12() {
-  // CHECK: define i32 @test12
-  // CHECK: ret i32 0
+  // O2: define i32 @test12
+  // O2: ret i32 0
   return __builtin_constant_p();
 }
 
 int test13() {
-  // CHECK: define i32 @test13
-  // CHECK: ret i32 1
+  // O2: define i32 @test13
+  // O2: ret i32 1
   return __builtin_constant_p( != 0);
 }
 
@@ -166,3 +167,13 @@
 
 extern char test16_v;
 struct { int a; } test16 = { __builtin_constant_p(test16_v) };
+
+extern unsigned long long test17_v;
+
+void test17() {
+  // O0: define void @test17
+  // O0: call void asm sideeffect "", {{.*}}(i32 -1) 
+  // O0: call void asm sideeffect "", {{.*}}(i32 -1) 
+  __asm__ __volatile__("" :: "n"( (__builtin_constant_p(test17_v) || 0) ? 1 : -1));
+  __asm__ __volatile__("" :: "i"( (__builtin_constant_p(test17_v) || 0) ? 1 : -1));
+}
Index: lib/CodeGen/CGStmt.cpp
===
--- lib/CodeGen/CGStmt.cpp
+++ lib/CodeGen/CGStmt.cpp
@@ -1821,7 +1821,7 @@
   // (immediate or symbolic), try to emit it as such.
   if (!Info.allowsRegister() && !Info.allowsMemory()) {
 Expr::EvalResult Result;
-if (InputExpr->EvaluateAsInt(Result, getContext()))
+if (InputExpr->EvaluateAsIntInConstantContext(Result, getContext()))
   return llvm::ConstantInt::get(getLLVMContext(), Result.Val.getInt());
 assert(!Info.requiresImmediateConstant() &&

[PATCH] D55616: Emit ASM input in a constant context

[Eli Friedman via Phabricator via cfe-commits]

efriedma added inline comments.



Comment at: lib/CodeGen/CGStmt.cpp:1825
+bool Success = false;
+if (CGM.getCodeGenOpts().OptimizationLevel > 0)
+  Success = InputExpr->EvaluateAsInt(Result, getContext());

Checking the optimization level here doesn't make sense. If a value is required 
to be constant in some context, we should evaluate it the same way at all 
optimization levels.

It's also important that the evaluation here computes the same result as 
Sema::ActOnGCCAsmStmt; otherwise you could hit an assertion failure.


Repository:
  rC Clang

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

https://reviews.llvm.org/D55616



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

[PATCH] D55616: Emit ASM input in a constant context

[Bill Wendling via Phabricator via cfe-commits]

void created this revision.
void added a reviewer: rsmith.
Herald added a subscriber: cfe-commits.
void added subscribers: jyknight, craig.topper.
void added a comment.

As a side note, the number of ways to evaluate a constant expression are legion 
in clang. They should really be unified...


Some ASM input constraints (e.g., "i" and "n") require immediate values. At O0,
very few code transformations are performed. So if we cannot resolve to an
immediate when emitting the ASM input we shouldn't delay its processing.


Repository:
  rC Clang

https://reviews.llvm.org/D55616

Files:
  include/clang/AST/Expr.h
  lib/AST/ExprConstant.cpp
  lib/CodeGen/CGStmt.cpp
  test/CodeGen/builtin-constant-p.c

Index: test/CodeGen/builtin-constant-p.c
===
--- test/CodeGen/builtin-constant-p.c
+++ test/CodeGen/builtin-constant-p.c
@@ -1,4 +1,5 @@
-// RUN: %clang_cc1 -triple x86_64-unknown-unknown -emit-llvm -o - %s -O2 | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -emit-llvm -o - %s -O2 | FileCheck --check-prefix=O2 %s
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -emit-llvm -o - %s -O0 | FileCheck --check-prefix=O0 %s
 
 int a = 42;
 
@@ -14,8 +15,8 @@
 struct foo f = (struct foo){ __builtin_constant_p(y), 42 };
 
 struct foo test0(int expr) {
-  // CHECK: define i64 @test0(i32 %expr)
-  // CHECK: call i1 @llvm.is.constant.i32(i32 %expr)
+  // O2: define i64 @test0(i32 %expr)
+  // O2: call i1 @llvm.is.constant.i32(i32 %expr)
   struct foo f = (struct foo){ __builtin_constant_p(expr), 42 };
   return f;
 }
@@ -27,15 +28,15 @@
 }
 
 int test1() {
-  // CHECK: define i32 @test1
-  // CHECK: add nsw i32 %0, -13
-  // CHECK-NEXT: call i1 @llvm.is.constant.i32(i32 %sub)
+  // O2: define i32 @test1
+  // O2: add nsw i32 %0, -13
+  // O2-NEXT: call i1 @llvm.is.constant.i32(i32 %sub)
   return bcp(test1_i() - 13);
 }
 
 int test2() {
-  // CHECK: define i32 @test2
-  // CHECK: ret i32 0
+  // O2: define i32 @test2
+  // O2: ret i32 0
   return __builtin_constant_p( - 13);
 }
 
@@ -44,8 +45,8 @@
 }
 
 int test3() {
-  // CHECK: define i32 @test3
-  // CHECK: ret i32 1
+  // O2: define i32 @test3
+  // O2: ret i32 1
   return bcp(test3_i() - 13);
 }
 
@@ -54,16 +55,16 @@
 int b[] = {1, 2, 3};
 
 int test4() {
-  // CHECK: define i32 @test4
-  // CHECK: ret i32 0
+  // O2: define i32 @test4
+  // O2: ret i32 0
   return __builtin_constant_p(b);
 }
 
 const char test5_c[] = {1, 2, 3, 0};
 
 int test5() {
-  // CHECK: define i32 @test5
-  // CHECK: ret i32 0
+  // O2: define i32 @test5
+  // O2: ret i32 0
   return __builtin_constant_p(test5_c);
 }
 
@@ -72,16 +73,16 @@
 }
 
 int test6() {
-  // CHECK: define i32 @test6
-  // CHECK: ret i32 0
+  // O2: define i32 @test6
+  // O2: ret i32 0
   return __builtin_constant_p(test6_i(test5_c));
 }
 
 /* --- Non-constant global variables */
 
 int test7() {
-  // CHECK: define i32 @test7
-  // CHECK: call i1 @llvm.is.constant.i32(i32 %0)
+  // O2: define i32 @test7
+  // O2: call i1 @llvm.is.constant.i32(i32 %0)
   return bcp(a);
 }
 
@@ -90,8 +91,8 @@
 const int c = 42;
 
 int test8() {
-  // CHECK: define i32 @test8
-  // CHECK: ret i32 1
+  // O2: define i32 @test8
+  // O2: ret i32 1
   return bcp(c);
 }
 
@@ -101,34 +102,34 @@
 const int c_arr[] = { 1, 2, 3 };
 
 int test9() {
-  // CHECK: define i32 @test9
-  // CHECK: call i1 @llvm.is.constant.i32(i32 %0)
+  // O2: define i32 @test9
+  // O2: call i1 @llvm.is.constant.i32(i32 %0)
   return __builtin_constant_p(arr[2]);
 }
 
 int test10() {
-  // CHECK: define i32 @test10
-  // CHECK: ret i32 1
+  // O2: define i32 @test10
+  // O2: ret i32 1
   return __builtin_constant_p(c_arr[2]);
 }
 
 int test11() {
-  // CHECK: define i32 @test11
-  // CHECK: ret i32 0
+  // O2: define i32 @test11
+  // O2: ret i32 0
   return __builtin_constant_p(c_arr);
 }
 
 /* --- Function pointers */
 
 int test12() {
-  // CHECK: define i32 @test12
-  // CHECK: ret i32 0
+  // O2: define i32 @test12
+  // O2: ret i32 0
   return __builtin_constant_p();
 }
 
 int test13() {
-  // CHECK: define i32 @test13
-  // CHECK: ret i32 1
+  // O2: define i32 @test13
+  // O2: ret i32 1
   return __builtin_constant_p( != 0);
 }
 
@@ -166,3 +167,13 @@
 
 extern char test16_v;
 struct { int a; } test16 = { __builtin_constant_p(test16_v) };
+
+extern unsigned long long test17_v;
+
+void test17() {
+  // O0: define void @test17
+  // O0: call void asm sideeffect "", {{.*}}(i32 -1) 
+  // O0: call void asm sideeffect "", {{.*}}(i32 -1) 
+  __asm__ __volatile__("" :: "n"( (__builtin_constant_p(test17_v) || 0) ? 1 : -1));
+  __asm__ __volatile__("" :: "i"( (__builtin_constant_p(test17_v) || 0) ? 1 : -1));
+}
Index: lib/CodeGen/CGStmt.cpp
===
--- lib/CodeGen/CGStmt.cpp
+++ lib/CodeGen/CGStmt.cpp
@@ -1821,7 +1821,12 @@
   // (immediate or symbolic), try to emit it as such.
   if (!Info.allowsRegister() && 

[PATCH] D55616: Emit ASM input in a constant context

[Bill Wendling via Phabricator via cfe-commits]

void added a comment.

As a side note, the number of ways to evaluate a constant expression are legion 
in clang. They should really be unified...


Repository:
  rC Clang

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

https://reviews.llvm.org/D55616



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