[clang] [llvm] [Clang] Repair the function "rParenEndsCast" to make incorrect judgments in template variable cases (PR #120904)

via llvm-commits llvm-commits at lists.llvm.org
Sun Dec 22 23:18:03 PST 2024


https://github.com/dty2 updated https://github.com/llvm/llvm-project/pull/120904

>From f36a48a92999cb791bf79b79adddaa73cab6f135 Mon Sep 17 00:00:00 2001
From: hunter <284050500 at qq.com>
Date: Mon, 23 Dec 2024 00:35:30 +0800
Subject: [PATCH 1/8] [Clang] Repair the functionrParenEndsCast to make
 incorrect judgments in template variable cases

---
 clang/lib/Format/TokenAnnotator.cpp | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/clang/lib/Format/TokenAnnotator.cpp b/clang/lib/Format/TokenAnnotator.cpp
index f2cfa7f49f62f9..fa9751cc8a7d92 100644
--- a/clang/lib/Format/TokenAnnotator.cpp
+++ b/clang/lib/Format/TokenAnnotator.cpp
@@ -38,6 +38,9 @@ static bool mustBreakAfterAttributes(const FormatToken &Tok,
 
 namespace {
 
+SmallVector<llvm::StringRef, 100> castIdentifiers{"__type_identity_t",
+                                                  "remove_reference_t"};
+
 /// Returns \c true if the line starts with a token that can start a statement
 /// with an initializer.
 static bool startsWithInitStatement(const AnnotatedLine &Line) {
@@ -2474,6 +2477,9 @@ class AnnotatingParser {
           Current.getNextNonComment()->isOneOf(tok::comma, tok::r_brace)) {
         Current.setType(TT_StringInConcatenation);
       }
+    } else if (Current.is(tok::kw_using)) {
+      if (Current.Next->Next->Next->isTypeName(LangOpts))
+        castIdentifiers.push_back(Current.Next->TokenText);
     } else if (Current.is(tok::l_paren)) {
       if (lParenStartsCppCast(Current))
         Current.setType(TT_CppCastLParen);
@@ -2831,8 +2837,18 @@ class AnnotatingParser {
         IsQualifiedPointerOrReference(BeforeRParen, LangOpts);
     bool ParensCouldEndDecl =
         AfterRParen->isOneOf(tok::equal, tok::semi, tok::l_brace, tok::greater);
-    if (ParensAreType && !ParensCouldEndDecl)
+    if (ParensAreType && !ParensCouldEndDecl) {
+      if (BeforeRParen->is(TT_TemplateCloser)) {
+        auto *Prev = BeforeRParen->MatchingParen->getPreviousNonComment();
+        if (Prev) {
+          for (auto &name : castIdentifiers)
+            if (Prev->TokenText == name)
+              return true;
+          return false;
+        }
+      }
       return true;
+    }
 
     // At this point, we heuristically assume that there are no casts at the
     // start of the line. We assume that we have found most cases where there

>From 3cdd6fddfbdbf5a27fa2c6cf3c26c57435c78b70 Mon Sep 17 00:00:00 2001
From: hunter <284050500 at qq.com>
Date: Mon, 23 Dec 2024 01:55:15 +0800
Subject: [PATCH 2/8] [Clang] Repair the functionrParenEndsCast to make
 incorrect judgments in template variable cases

---
 clang/lib/Format/TokenAnnotator.cpp | 1 +
 1 file changed, 1 insertion(+)

diff --git a/clang/lib/Format/TokenAnnotator.cpp b/clang/lib/Format/TokenAnnotator.cpp
index fa9751cc8a7d92..ccf18bbdea84e0 100644
--- a/clang/lib/Format/TokenAnnotator.cpp
+++ b/clang/lib/Format/TokenAnnotator.cpp
@@ -38,6 +38,7 @@ static bool mustBreakAfterAttributes(const FormatToken &Tok,
 
 namespace {
 
+// TODO: Add new Type modifiers
 SmallVector<llvm::StringRef, 100> castIdentifiers{"__type_identity_t",
                                                   "remove_reference_t"};
 

>From ae3dbe95fc9eaa4c5a6a59ad71cd6db845f68509 Mon Sep 17 00:00:00 2001
From: hunter <284050500 at qq.com>
Date: Mon, 23 Dec 2024 02:06:59 +0800
Subject: [PATCH 3/8] [Clang] Repair the functionrParenEndsCast to make
 incorrect judgments in template variable cases

---
 clang/lib/Format/TokenAnnotator.cpp | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/clang/lib/Format/TokenAnnotator.cpp b/clang/lib/Format/TokenAnnotator.cpp
index ccf18bbdea84e0..8b19e18a3d7ef5 100644
--- a/clang/lib/Format/TokenAnnotator.cpp
+++ b/clang/lib/Format/TokenAnnotator.cpp
@@ -17,6 +17,8 @@
 #include "clang/Basic/SourceManager.h"
 #include "clang/Basic/TokenKinds.h"
 #include "llvm/ADT/SmallPtrSet.h"
+#include "llvm/ADT/SmallVector.h"
+#include "llvm/ADT/StringRef.h"
 #include "llvm/Support/Debug.h"
 
 #define DEBUG_TYPE "format-token-annotator"

>From 78dcb7c5615c8e7524c6de11d3233690a6b09534 Mon Sep 17 00:00:00 2001
From: Simon Pilgrim <llvm-dev at redking.me.uk>
Date: Sun, 22 Dec 2024 16:40:00 +0000
Subject: [PATCH 4/8] [VectorCombine] foldShuffleToIdentity - add debug message
 for match

Helps with debugging to show to that the fold found the match.
---
 llvm/lib/Transforms/Vectorize/VectorCombine.cpp | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/llvm/lib/Transforms/Vectorize/VectorCombine.cpp b/llvm/lib/Transforms/Vectorize/VectorCombine.cpp
index b7956e037ad4b1..61f9b83b5c697c 100644
--- a/llvm/lib/Transforms/Vectorize/VectorCombine.cpp
+++ b/llvm/lib/Transforms/Vectorize/VectorCombine.cpp
@@ -2376,6 +2376,8 @@ bool VectorCombine::foldShuffleToIdentity(Instruction &I) {
   if (NumVisited <= 1)
     return false;
 
+  LLVM_DEBUG(dbgs() << "Found a superfluous identity shuffle: " << I << "\n");
+
   // If we got this far, we know the shuffles are superfluous and can be
   // removed. Scan through again and generate the new tree of instructions.
   Builder.SetInsertPoint(&I);

>From 65f9b378b8e2eba7c0ae08f2bec9308e86fd99f9 Mon Sep 17 00:00:00 2001
From: Simon Pilgrim <llvm-dev at redking.me.uk>
Date: Sun, 22 Dec 2024 17:21:29 +0000
Subject: [PATCH 5/8] [X86] IsNOT - don't fold not(pcmpgt(C1, C2)) ->
 pcmpgt(C2, C1 - 1)

Interferes with constant folding of the pcmpgt node.

Yes another example where topological node sorting would have helped us.

Fixes #120906
---
 llvm/lib/Target/X86/X86ISelLowering.cpp |  3 +-
 llvm/test/CodeGen/X86/pr120906.ll       | 40 +++++++++++++++++++++++++
 2 files changed, 42 insertions(+), 1 deletion(-)
 create mode 100644 llvm/test/CodeGen/X86/pr120906.ll

diff --git a/llvm/lib/Target/X86/X86ISelLowering.cpp b/llvm/lib/Target/X86/X86ISelLowering.cpp
index b50e0c60fadb64..3b260a89911c47 100644
--- a/llvm/lib/Target/X86/X86ISelLowering.cpp
+++ b/llvm/lib/Target/X86/X86ISelLowering.cpp
@@ -5246,7 +5246,8 @@ static SDValue IsNOT(SDValue V, SelectionDAG &DAG) {
     SmallVector<APInt> EltBits;
     if (getTargetConstantBitsFromNode(V.getOperand(0),
                                       V.getScalarValueSizeInBits(), UndefElts,
-                                      EltBits)) {
+                                      EltBits) &&
+        !ISD::isBuildVectorOfConstantSDNodes(V.getOperand(1).getNode())) {
       // Don't fold min_signed_value -> (min_signed_value - 1)
       bool MinSigned = false;
       for (APInt &Elt : EltBits) {
diff --git a/llvm/test/CodeGen/X86/pr120906.ll b/llvm/test/CodeGen/X86/pr120906.ll
new file mode 100644
index 00000000000000..f5f6331bf3bf63
--- /dev/null
+++ b/llvm/test/CodeGen/X86/pr120906.ll
@@ -0,0 +1,40 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py UTC_ARGS: --version 5
+; RUN: llc < %s -mtriple=x86_64-- | FileCheck %s
+
+define i32 @PR120906(ptr %p) {
+; CHECK-LABEL: PR120906:
+; CHECK:       # %bb.0:
+; CHECK-NEXT:    movl $564341309, (%rdi) # imm = 0x21A32A3D
+; CHECK-NEXT:    pxor %xmm0, %xmm0
+; CHECK-NEXT:    pxor %xmm1, %xmm1
+; CHECK-NEXT:    paddb %xmm1, %xmm1
+; CHECK-NEXT:    paddb %xmm1, %xmm1
+; CHECK-NEXT:    pxor %xmm2, %xmm2
+; CHECK-NEXT:    pcmpgtb %xmm1, %xmm2
+; CHECK-NEXT:    movdqa {{.*#+}} xmm1 = [11,11,11,11,u,u,u,u,u,u,u,u,u,u,u,u]
+; CHECK-NEXT:    movdqa %xmm1, %xmm3
+; CHECK-NEXT:    paddb %xmm1, %xmm3
+; CHECK-NEXT:    pand %xmm2, %xmm3
+; CHECK-NEXT:    pandn %xmm1, %xmm2
+; CHECK-NEXT:    por %xmm1, %xmm2
+; CHECK-NEXT:    por %xmm3, %xmm2
+; CHECK-NEXT:    punpcklbw {{.*#+}} xmm2 = xmm2[0],xmm0[0],xmm2[1],xmm0[1],xmm2[2],xmm0[2],xmm2[3],xmm0[3],xmm2[4],xmm0[4],xmm2[5],xmm0[5],xmm2[6],xmm0[6],xmm2[7],xmm0[7]
+; CHECK-NEXT:    punpcklwd {{.*#+}} xmm2 = xmm2[0],xmm0[0],xmm2[1],xmm0[1],xmm2[2],xmm0[2],xmm2[3],xmm0[3]
+; CHECK-NEXT:    pshufd {{.*#+}} xmm0 = xmm2[2,3,2,3]
+; CHECK-NEXT:    por %xmm2, %xmm0
+; CHECK-NEXT:    pshufd {{.*#+}} xmm1 = xmm0[1,1,1,1]
+; CHECK-NEXT:    por %xmm0, %xmm1
+; CHECK-NEXT:    movd %xmm1, %eax
+; CHECK-NEXT:    retq
+  store i32 564341309, ptr %p, align 4
+  %load = load i32, ptr %p, align 4
+  %broadcast.splatinsert.1 = insertelement <4 x i32> zeroinitializer, i32 %load, i64 0
+  %broadcast.splat.1 = shufflevector <4 x i32> %broadcast.splatinsert.1, <4 x i32> zeroinitializer, <4 x i32> zeroinitializer
+  %icmp = icmp ugt <4 x i32> %broadcast.splat.1, splat (i32 -9)
+  %zext8 = zext <4 x i1> %icmp to <4 x i8>
+  %shl = shl <4 x i8> splat (i8 11), %zext8
+  %or = or <4 x i8> %shl, splat (i8 11)
+  %zext32 = zext <4 x i8> %or to <4 x i32>
+  %rdx = tail call i32 @llvm.vector.reduce.or.v4i32(<4 x i32> %zext32)
+  ret i32 %rdx
+}

>From e1db51a954fea2d2a56fbf253462753148a03805 Mon Sep 17 00:00:00 2001
From: hunter <284050500 at qq.com>
Date: Mon, 23 Dec 2024 02:15:24 +0800
Subject: [PATCH 6/8] [Clang] Repair the functionrParenEndsCast to make
 incorrect judgments in template variable cases

---
 clang/lib/Format/TokenAnnotator.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/clang/lib/Format/TokenAnnotator.cpp b/clang/lib/Format/TokenAnnotator.cpp
index 8b19e18a3d7ef5..f3a7e591e8702c 100644
--- a/clang/lib/Format/TokenAnnotator.cpp
+++ b/clang/lib/Format/TokenAnnotator.cpp
@@ -41,7 +41,7 @@ static bool mustBreakAfterAttributes(const FormatToken &Tok,
 namespace {
 
 // TODO: Add new Type modifiers
-SmallVector<llvm::StringRef, 100> castIdentifiers{"__type_identity_t",
+llvm::SmallVector<llvm::StringRef, 100> castIdentifiers{"__type_identity_t",
                                                   "remove_reference_t"};
 
 /// Returns \c true if the line starts with a token that can start a statement

>From 8b1d1162378e7a11902b92a44bfdd75001ea6239 Mon Sep 17 00:00:00 2001
From: hunter <284050500 at qq.com>
Date: Mon, 23 Dec 2024 15:08:47 +0800
Subject: [PATCH 7/8] [Clang] Add unittests

---
 clang/lib/Format/TokenAnnotator.cpp           | 16 +++++++++-------
 clang/unittests/Format/TokenAnnotatorTest.cpp |  9 +++++++++
 2 files changed, 18 insertions(+), 7 deletions(-)

diff --git a/clang/lib/Format/TokenAnnotator.cpp b/clang/lib/Format/TokenAnnotator.cpp
index f3a7e591e8702c..da9860c7cd1e9f 100644
--- a/clang/lib/Format/TokenAnnotator.cpp
+++ b/clang/lib/Format/TokenAnnotator.cpp
@@ -41,7 +41,7 @@ static bool mustBreakAfterAttributes(const FormatToken &Tok,
 namespace {
 
 // TODO: Add new Type modifiers
-llvm::SmallVector<llvm::StringRef, 100> castIdentifiers{"__type_identity_t",
+llvm::SmallVector<llvm::StringRef> castIdentifiers{"__type_identity_t",
                                                   "remove_reference_t"};
 
 /// Returns \c true if the line starts with a token that can start a statement
@@ -2480,9 +2480,10 @@ class AnnotatingParser {
           Current.getNextNonComment()->isOneOf(tok::comma, tok::r_brace)) {
         Current.setType(TT_StringInConcatenation);
       }
-    } else if (Current.is(tok::kw_using)) {
-      if (Current.Next->Next->Next->isTypeName(LangOpts))
-        castIdentifiers.push_back(Current.Next->TokenText);
+    } else if (Style.isCpp() && Current.is(tok::kw_using)) {
+      if (Current.Next && Current.Next->Next && Current.Next->Next->Next)
+        if (Current.Next->Next->Next->isTypeName(LangOpts))
+          castIdentifiers.push_back(Current.Next->TokenText);
     } else if (Current.is(tok::l_paren)) {
       if (lParenStartsCppCast(Current))
         Current.setType(TT_CppCastLParen);
@@ -2841,9 +2842,10 @@ class AnnotatingParser {
     bool ParensCouldEndDecl =
         AfterRParen->isOneOf(tok::equal, tok::semi, tok::l_brace, tok::greater);
     if (ParensAreType && !ParensCouldEndDecl) {
-      if (BeforeRParen->is(TT_TemplateCloser)) {
-        auto *Prev = BeforeRParen->MatchingParen->getPreviousNonComment();
-        if (Prev) {
+      if (BeforeRParen->is(TT_TemplateCloser) ) {
+        if (determineUnaryOperatorByUsage(*AfterRParen)) return true;
+        if (AfterRParen->isOneOf(tok::plus, tok::minus, tok::star, tok::exclaim, tok::amp)) {
+          auto *Prev = BeforeRParen->MatchingParen->getPreviousNonComment();
           for (auto &name : castIdentifiers)
             if (Prev->TokenText == name)
               return true;
diff --git a/clang/unittests/Format/TokenAnnotatorTest.cpp b/clang/unittests/Format/TokenAnnotatorTest.cpp
index b2fb5227993c3f..7f7d5a46a9298c 100644
--- a/clang/unittests/Format/TokenAnnotatorTest.cpp
+++ b/clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -752,6 +752,15 @@ TEST_F(TokenAnnotatorTest, UnderstandsCasts) {
   ASSERT_EQ(Tokens.size(), 11u) << Tokens;
   EXPECT_TOKEN(Tokens[7], tok::r_paren, TT_CastRParen);
 
+  Tokens = annotate("(std::__type_identity_t<int>)-2;");
+  ASSERT_EQ(Tokens.size(), 12u) << Tokens;
+  EXPECT_TOKEN(Tokens[7], tok::r_paren, TT_CastRParen);
+
+  Tokens = annotate("template <typename> using type = int;\n"
+                    "auto = (type<int>)+5;");
+  ASSERT_EQ(Tokens.size(), 21u) << Tokens;
+  EXPECT_TOKEN(Tokens[16], tok::r_paren, TT_CastRParen);
+
   Tokens = annotate("return (Foo)p;");
   ASSERT_EQ(Tokens.size(), 7u) << Tokens;
   EXPECT_TOKEN(Tokens[3], tok::r_paren, TT_CastRParen);

>From 7971767d41bd333257fa51a040d522c9b40e3681 Mon Sep 17 00:00:00 2001
From: hunter <284050500 at qq.com>
Date: Mon, 23 Dec 2024 15:17:43 +0800
Subject: [PATCH 8/8] [Clang] Format

---
 clang/lib/Format/TokenAnnotator.cpp | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/clang/lib/Format/TokenAnnotator.cpp b/clang/lib/Format/TokenAnnotator.cpp
index da9860c7cd1e9f..3f44b8c18581e1 100644
--- a/clang/lib/Format/TokenAnnotator.cpp
+++ b/clang/lib/Format/TokenAnnotator.cpp
@@ -42,7 +42,7 @@ namespace {
 
 // TODO: Add new Type modifiers
 llvm::SmallVector<llvm::StringRef> castIdentifiers{"__type_identity_t",
-                                                  "remove_reference_t"};
+                                                   "remove_reference_t"};
 
 /// Returns \c true if the line starts with a token that can start a statement
 /// with an initializer.
@@ -2481,9 +2481,10 @@ class AnnotatingParser {
         Current.setType(TT_StringInConcatenation);
       }
     } else if (Style.isCpp() && Current.is(tok::kw_using)) {
-      if (Current.Next && Current.Next->Next && Current.Next->Next->Next)
+      if (Current.Next && Current.Next->Next && Current.Next->Next->Next) {
         if (Current.Next->Next->Next->isTypeName(LangOpts))
           castIdentifiers.push_back(Current.Next->TokenText);
+      }
     } else if (Current.is(tok::l_paren)) {
       if (lParenStartsCppCast(Current))
         Current.setType(TT_CppCastLParen);
@@ -2842,9 +2843,11 @@ class AnnotatingParser {
     bool ParensCouldEndDecl =
         AfterRParen->isOneOf(tok::equal, tok::semi, tok::l_brace, tok::greater);
     if (ParensAreType && !ParensCouldEndDecl) {
-      if (BeforeRParen->is(TT_TemplateCloser) ) {
-        if (determineUnaryOperatorByUsage(*AfterRParen)) return true;
-        if (AfterRParen->isOneOf(tok::plus, tok::minus, tok::star, tok::exclaim, tok::amp)) {
+      if (BeforeRParen->is(TT_TemplateCloser)) {
+        if (determineUnaryOperatorByUsage(*AfterRParen))
+          return true;
+        if (AfterRParen->isOneOf(tok::plus, tok::minus, tok::star, tok::exclaim,
+                                 tok::amp)) {
           auto *Prev = BeforeRParen->MatchingParen->getPreviousNonComment();
           for (auto &name : castIdentifiers)
             if (Prev->TokenText == name)



More information about the llvm-commits mailing list