glider created this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
glider added reviewers: eli.friedman, jyknight.
glider added a subscriber: vitalybuka.

The "=r" output constraint for a structure variable passed to inline asm
shouldn't be converted to "=*r", as this changes the asm directive
semantics and prevents DSE optimizations.
Instead, preserve the constraints and return such structures as integers
of corresponding size, which are converted back to structures when
storing the result.

Fixes PR42672.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D65234

Files:
  clang/lib/CodeGen/CGStmt.cpp
  clang/test/CodeGen/PR42672.c
  clang/test/CodeGen/asm-attrs.c

Index: clang/test/CodeGen/asm-attrs.c
===================================================================
--- clang/test/CodeGen/asm-attrs.c
+++ clang/test/CodeGen/asm-attrs.c
@@ -8,7 +8,7 @@
 // CHECK: call i32 asm "foo5", {{.*}} [[READONLY]]
 // CHECK: call i32 asm "foo6", {{.*}} [[NOATTRS]]
 // CHECK: call void asm sideeffect "foo7", {{.*}} [[NOATTRS]]
-// CHECK: call void asm "foo8", {{.*}} [[NOATTRS]]
+// CHECK: call i32 asm "foo8", {{.*}} [[READNONE]]
 
 // CHECK: attributes [[READNONE]] = { nounwind readnone }
 // CHECK: attributes [[NOATTRS]] = { nounwind }
Index: clang/test/CodeGen/PR42672.c
===================================================================
--- /dev/null
+++ clang/test/CodeGen/PR42672.c
@@ -0,0 +1,22 @@
+// RUN: %clang_cc1 -DSTRUCT -O2 -emit-llvm %s -o - | FileCheck %s
+// RUN: %clang_cc1 -USTRUCT -O2 -emit-llvm %s -o - | FileCheck %s
+//
+// Make sure Clang doesn't treat |lockval| as asm input, and therefore
+// optimizes away all stores to |lockval| together with the asm directive.
+void _raw_spin_lock(void) {
+#if STRUCT
+  struct {
+    unsigned short owner, next;
+  } lockval;
+  lockval.owner = 1;
+  lockval.next = 2;
+#else
+  int lockval;
+  lockval = 3;
+#endif
+  asm("nop"
+      : "=r"(lockval));
+}
+// CHECK-LABEL: _raw_spin_lock
+// CHECK-LABEL: entry:
+// CHECK-NEXT: ret void
Index: clang/lib/CodeGen/CGStmt.cpp
===================================================================
--- clang/lib/CodeGen/CGStmt.cpp
+++ clang/lib/CodeGen/CGStmt.cpp
@@ -1986,6 +1986,7 @@
   std::vector<llvm::Type *> ResultTruncRegTypes;
   std::vector<llvm::Type *> ArgTypes;
   std::vector<llvm::Value*> Args;
+  std::vector<bool> ResultTypeRequiresCast;
 
   // Keep track of inout constraints.
   std::string InOutConstraints;
@@ -2024,13 +2025,23 @@
 
     // If this is a register output, then make the inline asm return it
     // by-value.  If this is a memory result, return the value by-reference.
-    if (!Info.allowsMemory() && hasScalarEvaluationKind(OutExpr->getType())) {
+    bool isScalarizableAggregate =
+        hasAggregateEvaluationKind(OutExpr->getType());
+    if (!Info.allowsMemory() && (hasScalarEvaluationKind(OutExpr->getType()) ||
+                                 isScalarizableAggregate)) {
       Constraints += "=" + OutputConstraint;
       ResultRegQualTys.push_back(OutExpr->getType());
       ResultRegDests.push_back(Dest);
-      ResultRegTypes.push_back(ConvertTypeForMem(OutExpr->getType()));
-      ResultTruncRegTypes.push_back(ResultRegTypes.back());
-
+      ResultTruncRegTypes.push_back(ConvertTypeForMem(OutExpr->getType()));
+      if (Info.allowsRegister() && isScalarizableAggregate) {
+        ResultTypeRequiresCast.push_back(true);
+        unsigned Size = getContext().getTypeSize(OutExpr->getType());
+        llvm::Type *ConvTy = llvm::IntegerType::get(getLLVMContext(), Size);
+        ResultRegTypes.push_back(ConvTy);
+      } else {
+        ResultTypeRequiresCast.push_back(false);
+        ResultRegTypes.push_back(ResultTruncRegTypes.back());
+      }
       // If this output is tied to an input, and if the input is larger, then
       // we need to set the actual result type of the inline asm node to be the
       // same as the input type.
@@ -2273,6 +2284,7 @@
   assert(RegResults.size() == ResultRegTypes.size());
   assert(RegResults.size() == ResultTruncRegTypes.size());
   assert(RegResults.size() == ResultRegDests.size());
+  assert(ResultTypeRequiresCast.size() <= ResultRegDests.size());
   for (unsigned i = 0, e = RegResults.size(); i != e; ++i) {
     llvm::Value *Tmp = RegResults[i];
 
@@ -2302,7 +2314,17 @@
       }
     }
 
-    EmitStoreThroughLValue(RValue::get(Tmp), ResultRegDests[i]);
+    LValue Dest = ResultRegDests[i];
+    // ResultTypeRequiresCast elements correspond to the first
+    // ResultTypeRequiresCast.size() elements of RegResults.
+    if ((i < ResultTypeRequiresCast.size()) && ResultTypeRequiresCast[i]) {
+      unsigned Size = getContext().getTypeSize(ResultRegQualTys[i]);
+      Address A = Builder.CreateBitCast(Dest.getAddress(),
+                                        ResultRegTypes[i]->getPointerTo());
+      Dest = MakeAddrLValue(
+          A, getContext().getIntTypeForBitwidth(Size, /*Signed*/ false));
+    }
+    EmitStoreThroughLValue(RValue::get(Tmp), Dest);
   }
 }
 
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to