[PATCH] D107141: [Inline-asm] Add diagnosts for unsupported inline assembly arguments
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG32103608fc07: [Inline-asm] Add diagnosts for unsupported
inline assembly arguments (authored by pengfei).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D107141/new/
https://reviews.llvm.org/D107141
Files:
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/lib/CodeGen/CGStmt.cpp
clang/lib/Sema/SemaStmtAsm.cpp
clang/test/Sema/asm.c
Index: clang/test/Sema/asm.c
===
--- clang/test/Sema/asm.c
+++ clang/test/Sema/asm.c
@@ -313,3 +313,54 @@
asm ("jne %l0":::);
asm goto ("jne %l0"lab);
}
+
+typedef struct _st_size64 {
+ int a;
+ char b;
+} st_size64;
+
+typedef struct _st_size96 {
+ int a;
+ int b;
+ int c;
+} st_size96;
+
+typedef struct _st_size16 {
+ char a;
+ char b;
+} st_size16;
+
+typedef struct _st_size32 {
+ char a;
+ char b;
+ char c;
+ char d;
+} st_size32;
+
+typedef struct _st_size128 {
+ int a;
+ int b;
+ int c;
+ int d;
+} st_size128;
+
+void test19(long long x)
+{
+ st_size64 a;
+ st_size96 b;
+ st_size16 c;
+ st_size32 d;
+ st_size128 e;
+ asm ("" : "=rm" (a): "0" (1)); // no-error
+ asm ("" : "=rm" (d): "0" (1)); // no-error
+ asm ("" : "=rm" (c): "0" (x)); // no-error
+ // FIXME: This case is actually supported by codegen.
+ asm ("" : "=rm" (x): "0" (a)); // expected-error {{unsupported inline asm: input with type 'st_size64' (aka 'struct _st_size64') matching output with type 'long long'}}
+ // FIXME: This case is actually supported by codegen.
+ asm ("" : "=rm" (a): "0" (d)); // expected-error {{unsupported inline asm: input with type 'st_size32' (aka 'struct _st_size32') matching output with type 'st_size64' (aka 'struct _st_size64')}}
+ asm ("" : "=rm" (b): "0" (1)); // expected-error {{impossible constraint in asm: can't store value into a register}}
+ // FIXME: This case should be supported by codegen, but it fails now.
+ asm ("" : "=rm" (e): "0" (1)); // no-error
+ // FIXME: This case should be supported by codegen, but it fails now.
+ asm ("" : "=rm" (x): "0" (e)); // expected-error {{unsupported inline asm: input with type 'st_size128' (aka 'struct _st_size128') matching output with type 'long long'}}
+}
Index: clang/lib/Sema/SemaStmtAsm.cpp
===
--- clang/lib/Sema/SemaStmtAsm.cpp
+++ clang/lib/Sema/SemaStmtAsm.cpp
@@ -667,8 +667,17 @@
// output was a register, just extend the shorter one to the size of the
// larger one.
if (!SmallerValueMentioned && InputDomain != AD_Other &&
-OutputConstraintInfos[TiedTo].allowsRegister())
+OutputConstraintInfos[TiedTo].allowsRegister()) {
+ // FIXME: GCC supports the OutSize to be 128 at maximum. Currently codegen
+ // crash when the size larger than the register size. So we limit it here.
+ if (OutTy->isStructureType() &&
+ Context.getIntTypeForBitwidth(OutSize, /*Signed*/ false).isNull()) {
+targetDiag(OutputExpr->getExprLoc(), diag::err_store_value_to_reg);
+return NS;
+ }
+
continue;
+}
// Either both of the operands were mentioned or the smaller one was
// mentioned. One more special case that we'll allow: if the tied input is
Index: clang/lib/CodeGen/CGStmt.cpp
===
--- clang/lib/CodeGen/CGStmt.cpp
+++ clang/lib/CodeGen/CGStmt.cpp
@@ -2744,9 +2744,8 @@
QualType Ty = getContext().getIntTypeForBitwidth(Size, /*Signed*/ false);
if (Ty.isNull()) {
const Expr *OutExpr = S.getOutputExpr(i);
-CGM.Error(
-OutExpr->getExprLoc(),
-"impossible constraint in asm: can't store value into a register");
+CGM.getDiags().Report(OutExpr->getExprLoc(),
+ diag::err_store_value_to_reg);
return;
}
Dest = MakeAddrLValue(A, Ty);
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -8783,6 +8783,8 @@
" in asm %select{input|output}1 with a memory constraint '%2'">;
def err_asm_input_duplicate_match : Error<
"more than one input constraint matches the same output '%0'">;
+ def err_store_value_to_reg : Error<
+"impossible constraint in asm: can't store value into a register">;
def warn_asm_label_on_auto_decl : Warning<
"ignored asm label '%0' on automatic variable">;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D107141: [Inline-asm] Add diagnosts for unsupported inline assembly arguments
pengfei added a comment. Thanks @jyu2 ! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D107141/new/ https://reviews.llvm.org/D107141 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D107141: [Inline-asm] Add diagnosts for unsupported inline assembly arguments
jyu2 accepted this revision. jyu2 added a comment. This revision is now accepted and ready to land. This is okay with me. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D107141/new/ https://reviews.llvm.org/D107141 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D107141: [Inline-asm] Add diagnosts for unsupported inline assembly arguments
pengfei added inline comments.
Comment at: clang/lib/Sema/SemaStmtAsm.cpp:622
+if (InTy->isIntegerType() || InTy->isPointerType() ||
+InTy->isStructureType() || InTy->isConstantArrayType())
InputDomain = AD_Int;
jyu2 wrote:
> pengfei wrote:
> > jyu2 wrote:
> > > Are you sure you want to change the Input/output Domain? Since you
> > > changed this, could you add both codegen and sema check tests for struct
> > > type(you already has sema check for struct type, but I don't see any
> > > array type) and array type.
> > >
> > > Thanks.
> > > Jennifer
> > The input / output domain are just for sema check here. I don't think it
> > changes codegen behavior.
> Yes, for seam check, once error emit, compiler stop at CodeGen. However, if
> no error emit, compiler will go though the CodeGen.
>
> Since pointer/struct and constantArray are allowed for small size of type, so
> would you please add code gen test for that?
>
> In your test where no-error should emit part...
>
> asm ("" : "=rm" (a): "0" (1)); // no-error
>
> Thanks.
> Jennifer
Thanks Jennifer! I see your point. I'll only add sema check (rather than loose
in some way) in this patch.
The codegen part is complicated, some cases will pass but some still fail. I
have left comments in the tests.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D107141/new/
https://reviews.llvm.org/D107141
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D107141: [Inline-asm] Add diagnosts for unsupported inline assembly arguments
pengfei updated this revision to Diff 417303.
pengfei added a comment.
Address review comment.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D107141/new/
https://reviews.llvm.org/D107141
Files:
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/lib/CodeGen/CGStmt.cpp
clang/lib/Sema/SemaStmtAsm.cpp
clang/test/Sema/asm.c
Index: clang/test/Sema/asm.c
===
--- clang/test/Sema/asm.c
+++ clang/test/Sema/asm.c
@@ -313,3 +313,54 @@
asm ("jne %l0":::);
asm goto ("jne %l0"lab);
}
+
+typedef struct _st_size64 {
+ int a;
+ char b;
+} st_size64;
+
+typedef struct _st_size96 {
+ int a;
+ int b;
+ int c;
+} st_size96;
+
+typedef struct _st_size16 {
+ char a;
+ char b;
+} st_size16;
+
+typedef struct _st_size32 {
+ char a;
+ char b;
+ char c;
+ char d;
+} st_size32;
+
+typedef struct _st_size128 {
+ int a;
+ int b;
+ int c;
+ int d;
+} st_size128;
+
+void test19(long long x)
+{
+ st_size64 a;
+ st_size96 b;
+ st_size16 c;
+ st_size32 d;
+ st_size128 e;
+ asm ("" : "=rm" (a): "0" (1)); // no-error
+ asm ("" : "=rm" (d): "0" (1)); // no-error
+ asm ("" : "=rm" (c): "0" (x)); // no-error
+ // FIXME: This case is actually supported by codegen.
+ asm ("" : "=rm" (x): "0" (a)); // expected-error {{unsupported inline asm: input with type 'st_size64' (aka 'struct _st_size64') matching output with type 'long long'}}
+ // FIXME: This case is actually supported by codegen.
+ asm ("" : "=rm" (a): "0" (d)); // expected-error {{unsupported inline asm: input with type 'st_size32' (aka 'struct _st_size32') matching output with type 'st_size64' (aka 'struct _st_size64')}}
+ asm ("" : "=rm" (b): "0" (1)); // expected-error {{impossible constraint in asm: can't store value into a register}}
+ // FIXME: This case should be supported by codegen, but it fails now.
+ asm ("" : "=rm" (e): "0" (1)); // no-error
+ // FIXME: This case should be supported by codegen, but it fails now.
+ asm ("" : "=rm" (x): "0" (e)); // expected-error {{unsupported inline asm: input with type 'st_size128' (aka 'struct _st_size128') matching output with type 'long long'}}
+}
Index: clang/lib/Sema/SemaStmtAsm.cpp
===
--- clang/lib/Sema/SemaStmtAsm.cpp
+++ clang/lib/Sema/SemaStmtAsm.cpp
@@ -667,8 +667,17 @@
// output was a register, just extend the shorter one to the size of the
// larger one.
if (!SmallerValueMentioned && InputDomain != AD_Other &&
-OutputConstraintInfos[TiedTo].allowsRegister())
+OutputConstraintInfos[TiedTo].allowsRegister()) {
+ // FIXME: GCC supports the OutSize to be 128 at maximum. Currently codegen
+ // crash when the size larger than the register size. So we limit it here.
+ if (OutTy->isStructureType() &&
+ Context.getIntTypeForBitwidth(OutSize, /*Signed*/ false).isNull()) {
+targetDiag(OutputExpr->getExprLoc(), diag::err_store_value_to_reg);
+return NS;
+ }
+
continue;
+}
// Either both of the operands were mentioned or the smaller one was
// mentioned. One more special case that we'll allow: if the tied input is
Index: clang/lib/CodeGen/CGStmt.cpp
===
--- clang/lib/CodeGen/CGStmt.cpp
+++ clang/lib/CodeGen/CGStmt.cpp
@@ -2725,9 +2725,8 @@
QualType Ty = getContext().getIntTypeForBitwidth(Size, /*Signed*/ false);
if (Ty.isNull()) {
const Expr *OutExpr = S.getOutputExpr(i);
-CGM.Error(
-OutExpr->getExprLoc(),
-"impossible constraint in asm: can't store value into a register");
+CGM.getDiags().Report(OutExpr->getExprLoc(),
+ diag::err_store_value_to_reg);
return;
}
Dest = MakeAddrLValue(A, Ty);
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -8763,6 +8763,8 @@
" in asm %select{input|output}1 with a memory constraint '%2'">;
def err_asm_input_duplicate_match : Error<
"more than one input constraint matches the same output '%0'">;
+ def err_store_value_to_reg : Error<
+"impossible constraint in asm: can't store value into a register">;
def warn_asm_label_on_auto_decl : Warning<
"ignored asm label '%0' on automatic variable">;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D107141: [Inline-asm] Add diagnosts for unsupported inline assembly arguments
jyu2 added inline comments.
Comment at: clang/lib/Sema/SemaStmtAsm.cpp:622
+if (InTy->isIntegerType() || InTy->isPointerType() ||
+InTy->isStructureType() || InTy->isConstantArrayType())
InputDomain = AD_Int;
pengfei wrote:
> jyu2 wrote:
> > Are you sure you want to change the Input/output Domain? Since you changed
> > this, could you add both codegen and sema check tests for struct type(you
> > already has sema check for struct type, but I don't see any array type) and
> > array type.
> >
> > Thanks.
> > Jennifer
> The input / output domain are just for sema check here. I don't think it
> changes codegen behavior.
Yes, for seam check, once error emit, compiler stop at CodeGen. However, if no
error emit, compiler will go though the CodeGen.
Since pointer/struct and constantArray are allowed for small size of type, so
would you please add code gen test for that?
In your test where no-error should emit part...
asm ("" : "=rm" (a): "0" (1)); // no-error
Thanks.
Jennifer
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D107141/new/
https://reviews.llvm.org/D107141
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D107141: [Inline-asm] Add diagnosts for unsupported inline assembly arguments
pengfei added a comment. Ping @jyu2 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D107141/new/ https://reviews.llvm.org/D107141 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D107141: [Inline-asm] Add diagnosts for unsupported inline assembly arguments
pengfei added inline comments. Comment at: clang/lib/Sema/SemaStmtAsm.cpp:622 +if (InTy->isIntegerType() || InTy->isPointerType() || +InTy->isStructureType() || InTy->isConstantArrayType()) InputDomain = AD_Int; jyu2 wrote: > Are you sure you want to change the Input/output Domain? Since you changed > this, could you add both codegen and sema check tests for struct type(you > already has sema check for struct type, but I don't see any array type) and > array type. > > Thanks. > Jennifer The input / output domain are just for sema check here. I don't think it changes codegen behavior. Comment at: clang/lib/Sema/SemaStmtAsm.cpp:677 + Context.getIntTypeForBitwidth(OutSize, /*Signed*/ false).isNull()) +targetDiag(OutputExpr->getExprLoc(), diag::err_store_value_to_reg); + jyu2 wrote: > Do you need return NS after diagnostic? Good catch! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D107141/new/ https://reviews.llvm.org/D107141 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D107141: [Inline-asm] Add diagnosts for unsupported inline assembly arguments
pengfei updated this revision to Diff 414614.
pengfei added a comment.
Address review comments.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D107141/new/
https://reviews.llvm.org/D107141
Files:
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/lib/CodeGen/CGStmt.cpp
clang/lib/Sema/SemaStmtAsm.cpp
clang/test/Sema/asm.c
Index: clang/test/Sema/asm.c
===
--- clang/test/Sema/asm.c
+++ clang/test/Sema/asm.c
@@ -313,3 +313,51 @@
asm ("jne %l0":::);
asm goto ("jne %l0"lab);
}
+
+typedef struct _st_size64 {
+ int a;
+ char b;
+} st_size64;
+
+typedef struct _st_size96 {
+ int a;
+ int b;
+ int c;
+} st_size96;
+
+typedef struct _st_size16 {
+ char a;
+ char b;
+} st_size16;
+
+typedef struct _st_size32 {
+ char a;
+ char b;
+ char c;
+ char d;
+} st_size32;
+
+typedef struct _st_size128 {
+ int a;
+ int b;
+ int c;
+ int d;
+} st_size128;
+
+void test19(long long x)
+{
+ st_size64 a;
+ st_size96 b;
+ st_size16 c;
+ st_size32 d;
+ st_size128 e;
+ asm ("" : "=rm" (a): "0" (1)); // no-error
+ asm ("" : "=rm" (d): "0" (1)); // no-error
+ asm ("" : "=rm" (c): "0" (x)); // no-error
+ asm ("" : "=rm" (x): "0" (a)); // no-error
+ asm ("" : "=rm" (a): "0" (d)); // no-error
+ // Check the output size is pow of 2
+ asm ("" : "=rm" (b): "0" (1)); // expected-error {{impossible constraint in asm: can't store value into a register}}
+ asm ("" : "=rm" (e): "0" (1)); // no-error
+ asm ("" : "=rm" (x): "0" (e)); // no-error
+}
Index: clang/lib/Sema/SemaStmtAsm.cpp
===
--- clang/lib/Sema/SemaStmtAsm.cpp
+++ clang/lib/Sema/SemaStmtAsm.cpp
@@ -618,14 +618,16 @@
AD_Int, AD_FP, AD_Other
} InputDomain, OutputDomain;
-if (InTy->isIntegerType() || InTy->isPointerType())
+if (InTy->isIntegerType() || InTy->isPointerType() ||
+InTy->isStructureType())
InputDomain = AD_Int;
else if (InTy->isRealFloatingType())
InputDomain = AD_FP;
else
InputDomain = AD_Other;
-if (OutTy->isIntegerType() || OutTy->isPointerType())
+if (OutTy->isIntegerType() || OutTy->isPointerType() ||
+OutTy->isStructureType())
OutputDomain = AD_Int;
else if (OutTy->isRealFloatingType())
OutputDomain = AD_FP;
@@ -667,8 +669,17 @@
// output was a register, just extend the shorter one to the size of the
// larger one.
if (!SmallerValueMentioned && InputDomain != AD_Other &&
-OutputConstraintInfos[TiedTo].allowsRegister())
+OutputConstraintInfos[TiedTo].allowsRegister()) {
+ // FIXME: GCC supports the OutSize to be 128 at maximum. Currently codegen
+ // crash when the size larger than the register size. So we limit it here.
+ if (OutputDomain == AD_Int &&
+ Context.getIntTypeForBitwidth(OutSize, /*Signed*/ false).isNull()) {
+targetDiag(OutputExpr->getExprLoc(), diag::err_store_value_to_reg);
+return NS;
+ }
+
continue;
+}
// Either both of the operands were mentioned or the smaller one was
// mentioned. One more special case that we'll allow: if the tied input is
Index: clang/lib/CodeGen/CGStmt.cpp
===
--- clang/lib/CodeGen/CGStmt.cpp
+++ clang/lib/CodeGen/CGStmt.cpp
@@ -2725,9 +2725,8 @@
QualType Ty = getContext().getIntTypeForBitwidth(Size, /*Signed*/ false);
if (Ty.isNull()) {
const Expr *OutExpr = S.getOutputExpr(i);
-CGM.Error(
-OutExpr->getExprLoc(),
-"impossible constraint in asm: can't store value into a register");
+CGM.getDiags().Report(OutExpr->getExprLoc(),
+ diag::err_store_value_to_reg);
return;
}
Dest = MakeAddrLValue(A, Ty);
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -8763,6 +8763,8 @@
" in asm %select{input|output}1 with a memory constraint '%2'">;
def err_asm_input_duplicate_match : Error<
"more than one input constraint matches the same output '%0'">;
+ def err_store_value_to_reg : Error<
+"impossible constraint in asm: can't store value into a register">;
def warn_asm_label_on_auto_decl : Warning<
"ignored asm label '%0' on automatic variable">;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D107141: [Inline-asm] Add diagnosts for unsupported inline assembly arguments
jyu2 added inline comments. Comment at: clang/lib/Sema/SemaStmtAsm.cpp:622 +if (InTy->isIntegerType() || InTy->isPointerType() || +InTy->isStructureType() || InTy->isConstantArrayType()) InputDomain = AD_Int; Are you sure you want to change the Input/output Domain? Since you changed this, could you add both codegen and sema check tests for struct type(you already has sema check for struct type, but I don't see any array type) and array type. Thanks. Jennifer Comment at: clang/lib/Sema/SemaStmtAsm.cpp:677 + Context.getIntTypeForBitwidth(OutSize, /*Signed*/ false).isNull()) +targetDiag(OutputExpr->getExprLoc(), diag::err_store_value_to_reg); + Do you need return NS after diagnostic? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D107141/new/ https://reviews.llvm.org/D107141 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D107141: [Inline-asm] Add diagnosts for unsupported inline assembly arguments
pengfei updated this revision to Diff 414277.
pengfei added a comment.
Remove outdated comment
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D107141/new/
https://reviews.llvm.org/D107141
Files:
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/lib/CodeGen/CGStmt.cpp
clang/lib/Sema/SemaStmtAsm.cpp
clang/test/Sema/asm.c
Index: clang/test/Sema/asm.c
===
--- clang/test/Sema/asm.c
+++ clang/test/Sema/asm.c
@@ -313,3 +313,51 @@
asm ("jne %l0":::);
asm goto ("jne %l0"lab);
}
+
+typedef struct _st_size64 {
+ int a;
+ char b;
+} st_size64;
+
+typedef struct _st_size96 {
+ int a;
+ int b;
+ int c;
+} st_size96;
+
+typedef struct _st_size16 {
+ char a;
+ char b;
+} st_size16;
+
+typedef struct _st_size32 {
+ char a;
+ char b;
+ char c;
+ char d;
+} st_size32;
+
+typedef struct _st_size128 {
+ int a;
+ int b;
+ int c;
+ int d;
+} st_size128;
+
+void test19(long long x)
+{
+ st_size64 a;
+ st_size96 b;
+ st_size16 c;
+ st_size32 d;
+ st_size128 e;
+ asm ("" : "=rm" (a): "0" (1)); // no-error
+ asm ("" : "=rm" (d): "0" (1)); // no-error
+ asm ("" : "=rm" (c): "0" (x)); // no-error
+ asm ("" : "=rm" (x): "0" (a)); // no-error
+ asm ("" : "=rm" (a): "0" (d)); // no-error
+ // Check the output size is pow of 2
+ asm ("" : "=rm" (b): "0" (1)); // expected-error {{impossible constraint in asm: can't store value into a register}}
+ asm ("" : "=rm" (e): "0" (1)); // no-error
+ asm ("" : "=rm" (x): "0" (e)); // no-error
+}
Index: clang/lib/Sema/SemaStmtAsm.cpp
===
--- clang/lib/Sema/SemaStmtAsm.cpp
+++ clang/lib/Sema/SemaStmtAsm.cpp
@@ -618,14 +618,16 @@
AD_Int, AD_FP, AD_Other
} InputDomain, OutputDomain;
-if (InTy->isIntegerType() || InTy->isPointerType())
+if (InTy->isIntegerType() || InTy->isPointerType() ||
+InTy->isStructureType() || InTy->isConstantArrayType())
InputDomain = AD_Int;
else if (InTy->isRealFloatingType())
InputDomain = AD_FP;
else
InputDomain = AD_Other;
-if (OutTy->isIntegerType() || OutTy->isPointerType())
+if (OutTy->isIntegerType() || OutTy->isPointerType() ||
+OutTy->isStructureType() || OutTy->isConstantArrayType())
OutputDomain = AD_Int;
else if (OutTy->isRealFloatingType())
OutputDomain = AD_FP;
@@ -667,8 +669,15 @@
// output was a register, just extend the shorter one to the size of the
// larger one.
if (!SmallerValueMentioned && InputDomain != AD_Other &&
-OutputConstraintInfos[TiedTo].allowsRegister())
+OutputConstraintInfos[TiedTo].allowsRegister()) {
+ // FIXME: GCC supports the OutSize to be 128 at maximum. Currently codegen
+ // crash when the size larger than the register size. So we limit it here.
+ if (OutputDomain == AD_Int &&
+ Context.getIntTypeForBitwidth(OutSize, /*Signed*/ false).isNull())
+targetDiag(OutputExpr->getExprLoc(), diag::err_store_value_to_reg);
+
continue;
+}
// Either both of the operands were mentioned or the smaller one was
// mentioned. One more special case that we'll allow: if the tied input is
Index: clang/lib/CodeGen/CGStmt.cpp
===
--- clang/lib/CodeGen/CGStmt.cpp
+++ clang/lib/CodeGen/CGStmt.cpp
@@ -2725,9 +2725,8 @@
QualType Ty = getContext().getIntTypeForBitwidth(Size, /*Signed*/ false);
if (Ty.isNull()) {
const Expr *OutExpr = S.getOutputExpr(i);
-CGM.Error(
-OutExpr->getExprLoc(),
-"impossible constraint in asm: can't store value into a register");
+CGM.getDiags().Report(OutExpr->getExprLoc(),
+ diag::err_store_value_to_reg);
return;
}
Dest = MakeAddrLValue(A, Ty);
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -8763,6 +8763,8 @@
" in asm %select{input|output}1 with a memory constraint '%2'">;
def err_asm_input_duplicate_match : Error<
"more than one input constraint matches the same output '%0'">;
+ def err_store_value_to_reg : Error<
+"impossible constraint in asm: can't store value into a register">;
def warn_asm_label_on_auto_decl : Warning<
"ignored asm label '%0' on automatic variable">;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D107141: [Inline-asm] Add diagnosts for unsupported inline assembly arguments
pengfei added inline comments. Comment at: clang/lib/Sema/SemaStmtAsm.cpp:679 + !llvm::isPowerOf2_32(OutSize)) +targetDiag(OutputExpr->getExprLoc(), diag::err_store_value_to_reg); + jyu2 wrote: > Error message is not very clear to me. I think we should create more > specified error message there. Like power of two, or size < 8 or > pointer > size? > > Using error message selector. Use `getIntTypeForBitwidth` instead. So we don't need to check for each case. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D107141/new/ https://reviews.llvm.org/D107141 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D107141: [Inline-asm] Add diagnosts for unsupported inline assembly arguments
pengfei updated this revision to Diff 414276.
pengfei added a comment.
Address review comment.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D107141/new/
https://reviews.llvm.org/D107141
Files:
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/lib/CodeGen/CGStmt.cpp
clang/lib/Sema/SemaStmtAsm.cpp
clang/test/Sema/asm.c
Index: clang/test/Sema/asm.c
===
--- clang/test/Sema/asm.c
+++ clang/test/Sema/asm.c
@@ -313,3 +313,52 @@
asm ("jne %l0":::);
asm goto ("jne %l0"lab);
}
+
+typedef struct _st_size64 {
+ int a;
+ char b;
+} st_size64;
+
+typedef struct _st_size96 {
+ int a;
+ int b;
+ int c;
+} st_size96;
+
+typedef struct _st_size16 {
+ char a;
+ char b;
+} st_size16;
+
+typedef struct _st_size32 {
+ char a;
+ char b;
+ char c;
+ char d;
+} st_size32;
+
+typedef struct _st_size128 {
+ int a;
+ int b;
+ int c;
+ int d;
+} st_size128;
+
+void test19(long long x)
+{
+ st_size64 a;
+ st_size96 b;
+ st_size16 c;
+ st_size32 d;
+ st_size128 e;
+ asm ("" : "=rm" (a): "0" (1)); // no-error
+ asm ("" : "=rm" (d): "0" (1)); // no-error
+ asm ("" : "=rm" (c): "0" (x)); // no-error
+ asm ("" : "=rm" (x): "0" (a)); // no-error
+ asm ("" : "=rm" (a): "0" (d)); // no-error
+ // Check the output size is pow of 2
+ asm ("" : "=rm" (b): "0" (1)); // expected-error {{impossible constraint in asm: can't store value into a register}}
+ // Check the output size is <= 64
+ asm ("" : "=rm" (e): "0" (1)); // no-error
+ asm ("" : "=rm" (x): "0" (e)); // no-error
+}
Index: clang/lib/Sema/SemaStmtAsm.cpp
===
--- clang/lib/Sema/SemaStmtAsm.cpp
+++ clang/lib/Sema/SemaStmtAsm.cpp
@@ -618,14 +618,16 @@
AD_Int, AD_FP, AD_Other
} InputDomain, OutputDomain;
-if (InTy->isIntegerType() || InTy->isPointerType())
+if (InTy->isIntegerType() || InTy->isPointerType() ||
+InTy->isStructureType() || InTy->isConstantArrayType())
InputDomain = AD_Int;
else if (InTy->isRealFloatingType())
InputDomain = AD_FP;
else
InputDomain = AD_Other;
-if (OutTy->isIntegerType() || OutTy->isPointerType())
+if (OutTy->isIntegerType() || OutTy->isPointerType() ||
+OutTy->isStructureType() || OutTy->isConstantArrayType())
OutputDomain = AD_Int;
else if (OutTy->isRealFloatingType())
OutputDomain = AD_FP;
@@ -667,8 +669,15 @@
// output was a register, just extend the shorter one to the size of the
// larger one.
if (!SmallerValueMentioned && InputDomain != AD_Other &&
-OutputConstraintInfos[TiedTo].allowsRegister())
+OutputConstraintInfos[TiedTo].allowsRegister()) {
+ // FIXME: GCC supports the OutSize to be 128 at maximum. Currently codegen
+ // crash when the size larger than the register size. So we limit it here.
+ if (OutputDomain == AD_Int &&
+ Context.getIntTypeForBitwidth(OutSize, /*Signed*/ false).isNull())
+targetDiag(OutputExpr->getExprLoc(), diag::err_store_value_to_reg);
+
continue;
+}
// Either both of the operands were mentioned or the smaller one was
// mentioned. One more special case that we'll allow: if the tied input is
Index: clang/lib/CodeGen/CGStmt.cpp
===
--- clang/lib/CodeGen/CGStmt.cpp
+++ clang/lib/CodeGen/CGStmt.cpp
@@ -2725,9 +2725,8 @@
QualType Ty = getContext().getIntTypeForBitwidth(Size, /*Signed*/ false);
if (Ty.isNull()) {
const Expr *OutExpr = S.getOutputExpr(i);
-CGM.Error(
-OutExpr->getExprLoc(),
-"impossible constraint in asm: can't store value into a register");
+CGM.getDiags().Report(OutExpr->getExprLoc(),
+ diag::err_store_value_to_reg);
return;
}
Dest = MakeAddrLValue(A, Ty);
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -8763,6 +8763,8 @@
" in asm %select{input|output}1 with a memory constraint '%2'">;
def err_asm_input_duplicate_match : Error<
"more than one input constraint matches the same output '%0'">;
+ def err_store_value_to_reg : Error<
+"impossible constraint in asm: can't store value into a register">;
def warn_asm_label_on_auto_decl : Warning<
"ignored asm label '%0' on automatic variable">;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D107141: [Inline-asm] Add diagnosts for unsupported inline assembly arguments
jyu2 added inline comments. Comment at: clang/lib/Sema/SemaStmtAsm.cpp:679 + !llvm::isPowerOf2_32(OutSize)) +targetDiag(OutputExpr->getExprLoc(), diag::err_store_value_to_reg); + Error message is not very clear to me. I think we should create more specified error message there. Like power of two, or size < 8 or > pointer size? Using error message selector. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D107141/new/ https://reviews.llvm.org/D107141 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D107141: [Inline-asm] Add diagnosts for unsupported inline assembly arguments
pengfei added a comment. Ping @jyu2 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D107141/new/ https://reviews.llvm.org/D107141 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D107141: [Inline-asm] Add diagnosts for unsupported inline assembly arguments
pengfei updated this revision to Diff 413676.
pengfei marked 2 inline comments as done.
pengfei added a comment.
Thanks @skan !
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D107141/new/
https://reviews.llvm.org/D107141
Files:
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/lib/CodeGen/CGStmt.cpp
clang/lib/Sema/SemaStmtAsm.cpp
clang/test/Sema/asm.c
Index: clang/test/Sema/asm.c
===
--- clang/test/Sema/asm.c
+++ clang/test/Sema/asm.c
@@ -1,4 +1,5 @@
// RUN: %clang_cc1 %s -Wno-private-extern -triple i386-pc-linux-gnu -verify -fsyntax-only
+// RUN: %clang_cc1 %s -Wno-private-extern -triple x86_64-pc-linux-gnu -verify -fsyntax-only -DTEST_X86_64
void f(void) {
@@ -313,3 +314,76 @@
asm ("jne %l0":::);
asm goto ("jne %l0"lab);
}
+
+typedef struct _st_size64 {
+ int a;
+ char b;
+} st_size64;
+
+typedef struct _st_size96 {
+ int a;
+ int b;
+ int c;
+} st_size96;
+
+typedef struct _st_size16 {
+ char a;
+ char b;
+} st_size16;
+
+typedef struct _st_size32 {
+ char a;
+ char b;
+ char c;
+ char d;
+} st_size32;
+
+typedef struct _st_size128 {
+ int a;
+ int b;
+ int c;
+ int d;
+} st_size128;
+
+#ifdef TEST_X86_64
+void test19(long long x)
+{
+ st_size64 a;
+ st_size96 b;
+ st_size16 c;
+ st_size32 d;
+ st_size128 e;
+ asm ("" : "=rm" (a): "0" (1)); // no-error
+ asm ("" : "=rm" (d): "0" (1)); // no-error
+ asm ("" : "=rm" (c): "0" (x)); // no-error
+ asm ("" : "=rm" (x): "0" (a)); // no-error
+ asm ("" : "=rm" (a): "0" (d)); // no-error
+ // Check the output size is pow of 2
+ asm ("" : "=rm" (b): "0" (1)); // expected-error {{impossible constraint in asm: can't store value into a register}}
+ // Check the output size is <= 64
+ asm ("" : "=rm" (e): "0" (1)); // expected-error {{impossible constraint in asm: can't store value into a register}}
+ asm ("" : "=rm" (x): "0" (e)); // no-error
+}
+#else
+void test19(long long x)
+{
+ st_size64 a;
+ st_size96 b;
+ st_size16 c;
+ st_size32 d;
+ st_size128 e;
+ // Check the output size is <= 32
+ asm ("" : "=rm" (a): "0" (1)); // expected-error {{impossible constraint in asm: can't store value into a register}}
+ asm ("" : "=rm" (d): "0" (1)); // no-error
+ asm ("" : "=rm" (c): "0" (x)); // no-error
+ asm ("" : "=rm" (x): "0" (a)); // no-error
+ // Check the output size is <= 32
+ asm ("" : "=rm" (a): "0" (d)); // expected-error {{impossible constraint in asm: can't store value into a register}}
+ // Check the output size is pow of 2
+ asm ("" : "=rm" (b): "0" (1)); // expected-error {{impossible constraint in asm: can't store value into a register}}
+ // Check the output size is <= 32
+ asm ("" : "=rm" (e): "0" (1)); // expected-error {{impossible constraint in asm: can't store value into a register}}
+ // Check the output size is <= 32
+ asm ("" : "=rm" (x): "0" (e)); // expected-error {{impossible constraint in asm: can't store value into a register}}
+}
+#endif
Index: clang/lib/Sema/SemaStmtAsm.cpp
===
--- clang/lib/Sema/SemaStmtAsm.cpp
+++ clang/lib/Sema/SemaStmtAsm.cpp
@@ -618,14 +618,16 @@
AD_Int, AD_FP, AD_Other
} InputDomain, OutputDomain;
-if (InTy->isIntegerType() || InTy->isPointerType())
+if (InTy->isIntegerType() || InTy->isPointerType() ||
+InTy->isStructureType() || InTy->isConstantArrayType())
InputDomain = AD_Int;
else if (InTy->isRealFloatingType())
InputDomain = AD_FP;
else
InputDomain = AD_Other;
-if (OutTy->isIntegerType() || OutTy->isPointerType())
+if (OutTy->isIntegerType() || OutTy->isPointerType() ||
+OutTy->isStructureType() || OutTy->isConstantArrayType())
OutputDomain = AD_Int;
else if (OutTy->isRealFloatingType())
OutputDomain = AD_FP;
@@ -667,8 +669,17 @@
// output was a register, just extend the shorter one to the size of the
// larger one.
if (!SmallerValueMentioned && InputDomain != AD_Other &&
-OutputConstraintInfos[TiedTo].allowsRegister())
+OutputConstraintInfos[TiedTo].allowsRegister()) {
+ // FIXME: GCC supports the OutSize to be 128 at maximum. Currently codegen
+ // crash when the size larger than the register size. So we limit it here.
+ if (OutputDomain != AD_Int)
+continue;
+ if (OutSize < 8 || OutSize > Context.getTypeSize(Context.getSizeType()) ||
+ !llvm::isPowerOf2_32(OutSize))
+targetDiag(OutputExpr->getExprLoc(), diag::err_store_value_to_reg);
+
continue;
+}
// Either both of the operands were mentioned or the smaller one was
// mentioned. One more special case that we'll allow: if the tied input is
Index: clang/lib/CodeGen/CGStmt.cpp
===
--- clang/lib/CodeGen/CGStmt.cpp
+++ clang/lib/CodeGen/CGStmt.cpp
@@
[PATCH] D107141: [Inline-asm] Add diagnosts for unsupported inline assembly arguments
skan added inline comments.
Comment at: clang/test/Sema/asm.c:318-346
+typedef struct test19_a {
+ int a;
+ char b;
+} test19_a;
+
+typedef struct test19_b {
+ int a;
Better to add comments about the size of the struct.
Comment at: clang/test/Sema/asm.c:361-362
+ asm ("" : "=rm" (a): "0" (d)); // no-error
+ asm ("" : "=rm" (b): "0" (1)); // expected-error {{impossible constraint in
asm: can't store value into a register}}
+ asm ("" : "=rm" (e): "0" (1)); // expected-error {{impossible constraint in
asm: can't store value into a register}}
+ asm ("" : "=rm" (x): "0" (e)); // no-error
Better to comment that we are checking the size is power of 2 here.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D107141/new/
https://reviews.llvm.org/D107141
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
