From 2f2fee618be7d5828acf3fde71ede525d8f2e9bc Mon Sep 17 00:00:00 2001
From: Andi Drebes <andi@drebesium.org>
Date: Mon, 27 May 2024 12:41:13 +0200
Subject: [PATCH] Fix sign handling of alias types for binary operators and
 casts

The generated IR for some of the AST nodes handled by
`MLIRScanner::VisitBinaryOperator()` and
`MLIRScanner::VisitCastExpr()` depends on the sign of the operand or
result types. Some of these checks attempt to directly cast the result
of `Expr::getType()` to `BuiltinType` and then check if the resulting
`QualType` is a signed or unsigned integer type. If the cast fails,
the type is assumed to be signed.

However, the type being checked may not directly be a builtin type
(e.g., it may be an alias for an unsigned type wrapped in a
`TypedefType`) and the default assumption may not hold.

Examining the canonical type retrieved via
`QualType::getCanonicalType()`, as implemented by this change, solves
the issue.
---
 tools/cgeist/Lib/clang-mlir.cc                | 15 ++++++---
 .../Test/Verification/unsigned-type-alias.c   | 32 +++++++++++++++++++
 2 files changed, 42 insertions(+), 5 deletions(-)
 create mode 100644 tools/cgeist/Test/Verification/unsigned-type-alias.c

diff --git a/tools/cgeist/Lib/clang-mlir.cc b/tools/cgeist/Lib/clang-mlir.cc
index 058464c323ef..082e098b094d 100644
--- a/tools/cgeist/Lib/clang-mlir.cc
+++ b/tools/cgeist/Lib/clang-mlir.cc
@@ -2564,7 +2564,8 @@ ValueCategory MLIRScanner::VisitBinaryOperator(clang::BinaryOperator *BO) {
     auto prevTy = res.getType().cast<mlir::IntegerType>();
     auto postTy = getMLIRType(BO->getType()).cast<mlir::IntegerType>();
     bool signedType = true;
-    if (auto bit = dyn_cast<clang::BuiltinType>(&*BO->getType())) {
+    if (auto bit =
+            dyn_cast<clang::BuiltinType>(&*BO->getType().getCanonicalType())) {
       if (bit->isUnsignedInteger())
         signedType = false;
       if (bit->isSignedInteger())
@@ -2690,7 +2691,8 @@ ValueCategory MLIRScanner::VisitBinaryOperator(clang::BinaryOperator *BO) {
   }
   // TODO note assumptions made here about unsigned / unordered
   bool signedType = true;
-  if (auto bit = dyn_cast<clang::BuiltinType>(&*BO->getType())) {
+  if (auto bit =
+          dyn_cast<clang::BuiltinType>(&*BO->getType().getCanonicalType())) {
     if (bit->isUnsignedInteger())
       signedType = false;
     if (bit->isSignedInteger())
@@ -2763,7 +2765,8 @@ ValueCategory MLIRScanner::VisitBinaryOperator(clang::BinaryOperator *BO) {
   case clang::BinaryOperator::Opcode::BO_EQ:
   case clang::BinaryOperator::Opcode::BO_NE: {
     signedType = true;
-    if (auto bit = dyn_cast<clang::BuiltinType>(&*BO->getLHS()->getType())) {
+    if (auto bit = dyn_cast<clang::BuiltinType>(
+            &*BO->getLHS()->getType().getCanonicalType())) {
       if (bit->isUnsignedInteger())
         signedType = false;
       if (bit->isSignedInteger())
@@ -2998,7 +3001,8 @@ ValueCategory MLIRScanner::VisitBinaryOperator(clang::BinaryOperator *BO) {
       if (auto prevTy = dyn_cast<mlir::IntegerType>(tostore.getType())) {
         if (auto postTy = dyn_cast<mlir::IntegerType>(subType)) {
           bool signedType = true;
-          if (auto bit = dyn_cast<clang::BuiltinType>(&*BO->getType())) {
+          if (auto bit = dyn_cast<clang::BuiltinType>(
+                  &*BO->getType().getCanonicalType())) {
             if (bit->isUnsignedInteger())
               signedType = false;
             if (bit->isSignedInteger())
@@ -4174,7 +4178,8 @@ ValueCategory MLIRScanner::VisitCastExpr(CastExpr *E) {
     }
     auto prevTy = scalar.getType().cast<mlir::IntegerType>();
     bool signedType = true;
-    if (auto bit = dyn_cast<clang::BuiltinType>(&*E->getSubExpr()->getType())) {
+    if (auto bit = dyn_cast<clang::BuiltinType>(
+            &*E->getSubExpr()->getType().getCanonicalType())) {
       if (bit->isUnsignedInteger())
         signedType = false;
       if (bit->isSignedInteger())
diff --git a/tools/cgeist/Test/Verification/unsigned-type-alias.c b/tools/cgeist/Test/Verification/unsigned-type-alias.c
new file mode 100644
index 000000000000..b6995cf2c607
--- /dev/null
+++ b/tools/cgeist/Test/Verification/unsigned-type-alias.c
@@ -0,0 +1,32 @@
+// RUN: cgeist %s --function=* -S | FileCheck %s
+
+typedef unsigned int ui32;
+typedef unsigned long long ui64;
+
+// CHECK:      func.func @shift(%[[Varg0:.*]]: i64, %[[Varg1:.*]]: i64) -> i64
+// CHECK-NEXT:   %[[V0:.*]] = arith.shrui %[[Varg0]], %[[Varg1]] : i64
+// CHECK-NEXT:   return %[[V0]] : i64
+// CHECK-NEXT: }
+
+ui64 shift(ui64 input, ui64 shift) {
+  return input >> shift;
+}
+
+// CHECK:      func.func @ge(%[[Varg0:.*]]: i64, %[[Varg1:.*]]: i64) -> i32
+// CHECK-NEXT:   %[[V0:.*]] = arith.cmpi ugt, %[[Varg0]], %[[Varg1]] : i64
+// CHECK-NEXT:   %[[V1:.*]] = arith.extui %[[V0]] : i1 to i32
+// CHECK-NEXT:   return %[[V1]] : i32
+// CHECK-NEXT: }
+
+int ge(ui64 input, ui64 v) {
+  return input > v;
+}
+
+// CHECK-NEXT:   func.func @ret(%[[Varg0:.*]]: i32) -> i64
+// CHECK-NEXT:     %[[V0:.*]] = arith.extui %[[Varg0]] : i32 to i64
+// CHECK-NEXT:     return %[[V0]] : i64
+// CHECK-NEXT:   }
+
+ui64 ret(ui32 input) {
+  return input;
+}