[PATCH] D110808: [APInt] Stop using soft-deprecated constructors and methods in clang. NFC.

2021-10-03 Thread Chris Lattner via Phabricator via cfe-commits 
lattner accepted this revision.
lattner added a comment.
This revision is now accepted and ready to land.

Thank you Jay!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110808

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

[PATCH] D110833: [clang-format] Add ControlStatementsAndFunctionDefinitionsExceptControlMacros option to SpaceBeforeParens

2021-10-03 Thread Owen Pan via Phabricator via cfe-commits 
owenpan added inline comments.



Comment at: clang/docs/ClangFormatStyleOptions.rst:3649
 ``SBPO_ControlStatementsExceptForEachMacros`` remains an alias for
 backward compatibility.
 

HazardyKnusperkeks wrote:
> crayroud wrote:
> > MyDeveloperDay wrote:
> > > Now I look back here, why where these Macro considered the same as for 
> > > loops? why would we want
> > > 
> > > ```
> > > for ()
> > > Q_FOREACH(...)
> > > ```
> > > 
> > > So this really does need a struct or we'll be eventually be adding
> > > 
> > > `SBPO_ControlStatementsAndFunctionDefinitionsExceptControlMacrosButNotIfAndDefinatelyWhilesAndSometimesSwitchButOnlyOnTheSecondThursdayOfTheMonth`
> > > 
> > > ```
> > > SpaceBeforeParen:
> > >  AfterFunctionDefinitionName: false
> > >  AfterFunctionDeclarationName: true
> > >  AfterSwitch: true
> > >  AfterForeachMacros: false
> > >   (there are likely others)
> > > ```
> > >  
> > > `
> > > 
> > > 
> > > 
> > > 
> > > 
> > Indeed replacing the enum with a struct as suggested is better to support 
> > the different possible combinations, compare to the current version of 
> > SpaceBeforeParens that results in very long names.
> > 
> > To support existing configuration files, I propose to keep the enum and to 
> > add a struct to handle the custom use cases and to cleanup the code. What 
> > do you think ?
> > 
> > ```
> > SpaceBeforeParens: Custom
> > SpaceBeforeParensCustom:
> >  AfterFunctionDefinitionName: false
> >  AfterFunctionDeclarationName: true
> >  AfterSwitch: true
> >  AfterForeachMacros: false
> > …
> > ```
> I haven't looked too deep into the parsing, but if we could try to parse it 
> as a struct and if that fails as enum for compatibility I would be in favor 
> of that. But a `custom` is also acceptable.
If possible, we should deprecate the enum values and match them to the new 
struct values for backward compatibility without resorting to a `Custom` 
sub-option. See `PackConstructorInitializers` for an example.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110833

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

[PATCH] D111009: Update inline builtin handling to honor gnu inline attribute

2021-10-03 Thread Nick Desaulniers via Phabricator via cfe-commits 
nickdesaulniers added a comment.

Diff 376794 builds! Let's still add a test for

  inline __attribute__((__always_inline__)) __attribute__((gnu_inline)) void*
  memcpy() {}

though.


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

https://reviews.llvm.org/D111009

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

[PATCH] D111009: Update inline builtin handling to honor gnu inline attribute

2021-10-03 Thread Nick Desaulniers via Phabricator via cfe-commits 
nickdesaulniers added a comment.

With Diff 376790

  inline __attribute__((__always_inline__)) __attribute__((gnu_inline)) void*
  memcpy() {}

`clang -O2` is producing undefined references to `memcpy.inline`. Please add 
that to the new unit tests added here.

  diff --git a/clang/lib/CodeGen/CodeGenFunction.cpp 
b/clang/lib/CodeGen/CodeGenFunction.cpp
  index a8ae5f5fc0de..132834516473 100644
  --- a/clang/lib/CodeGen/CodeGenFunction.cpp
  +++ b/clang/lib/CodeGen/CodeGenFunction.cpp
  @@ -1303,7 +1303,7 @@ void CodeGenFunction::GenerateCode(GlobalDecl GD, 
llvm::Function *Fn,
   llvm::Module *M = Fn->getParent();
   llvm::Function *Clone = M->getFunction(FDInlineName);
   if (!Clone) {
  -  Clone = llvm::Function::Create(Fn->getFunctionType(), Fn->getLinkage(),
  +  Clone = llvm::Function::Create(Fn->getFunctionType(), 
llvm::GlobalValue::AvailableExternallyLinkage,
Fn->getAddressSpace(), FDInlineName, M);
 Clone->addFnAttr(llvm::Attribute::AlwaysInline);
   }

fixes that, but I don't think that's the right visibility.

  02a0 T memcpy.inline

should be `t` not `T` I think?  Because now the failure looks like

  ld.lld: error: duplicate symbol: memcpy.inline
  >>> defined at file.c
  >>>fs/efivarfs/file.o:(memcpy.inline)
  >>> defined at super.c
  >>>fs/efivarfs/super.o:(.text+0x800)

Perhaps `llvm::GlobalValue::InternalLinkage`?


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

https://reviews.llvm.org/D111009

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

[PATCH] D111009: Update inline builtin handling to honor gnu inline attribute

2021-10-03 Thread serge via Phabricator via cfe-commits 
serge-sans-paille updated this revision to Diff 376794.
serge-sans-paille added a comment.

+ fix linkage of generated function


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

https://reviews.llvm.org/D111009

Files:
  clang/lib/AST/Decl.cpp
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/test/CodeGen/memcmp-inline-builtin-to-asm.c
  clang/test/CodeGen/memcpy-inline-builtin.c
  clang/test/CodeGen/memcpy-nobuiltin.c
  clang/test/CodeGen/memcpy-nobuiltin.inc
  clang/test/CodeGen/pr9614.c

Index: clang/test/CodeGen/pr9614.c
===
--- clang/test/CodeGen/pr9614.c
+++ clang/test/CodeGen/pr9614.c
@@ -32,14 +32,14 @@
 
 // CHECK-LABEL: define{{.*}} void @f()
 // CHECK: call void @foo()
-// CHECK: call i32 @abs(i32 %0)
+// CHECK: call i32 @abs(i32 0)
 // CHECK: call i8* @strrchr(
 // CHECK: call void @llvm.prefetch.p0i8(
 // CHECK: call i8* @memchr(
 // CHECK: ret void
 
 // CHECK: declare void @foo()
+// CHECK: declare i32 @abs(i32
 // CHECK: declare i8* @strrchr(i8*, i32)
 // CHECK: declare i8* @memchr(
-// CHECK: declare i32 @abs(i32
 // CHECK: declare void @llvm.prefetch.p0i8(
Index: clang/test/CodeGen/memcpy-nobuiltin.inc
===
--- clang/test/CodeGen/memcpy-nobuiltin.inc
+++ clang/test/CodeGen/memcpy-nobuiltin.inc
@@ -2,7 +2,7 @@
 extern void *memcpy(void *dest, void const *from, size_t n);
 
 #ifdef WITH_DECL
-inline __attribute__((always_inline)) void *memcpy(void *dest, void const *from, size_t n) {
+inline __attribute__((always_inline)) __attribute__((gnu_inline)) void *memcpy(void *dest, void const *from, size_t n) {
   char const *ifrom = from;
   char *idest = dest;
   while (n--)
@@ -11,7 +11,7 @@
 }
 #endif
 #ifdef WITH_SELF_REFERENCE_DECL
-inline __attribute__((always_inline)) void *memcpy(void *dest, void const *from, size_t n) {
+inline __attribute__((always_inline)) __attribute__((gnu_inline)) void *memcpy(void *dest, void const *from, size_t n) {
   if (n != 0)
 memcpy(dest, from, n);
   return dest;
Index: clang/test/CodeGen/memcpy-nobuiltin.c
===
--- clang/test/CodeGen/memcpy-nobuiltin.c
+++ clang/test/CodeGen/memcpy-nobuiltin.c
@@ -1,11 +1,11 @@
 // RUN: %clang_cc1 -verify -S -emit-llvm -o- %s -isystem %S -DWITH_DECL | FileCheck --check-prefix=CHECK-WITH-DECL %s
 // RUN: %clang_cc1 -verify -S -emit-llvm -o- %s -isystem %S -UWITH_DECL | FileCheck --check-prefix=CHECK-NO-DECL %s
-// RUN: %clang_cc1 -verify -S -emit-llvm -o- %s -isystem %S -DWITH_SELF_REFERENCE_DECL | FileCheck --check-prefix=CHECK-SELF-REF-DECL %s
+// RUN: %clang_cc1 -verify -S -emit-llvm -o- %s -isystem %S -disable-llvm-passes -DWITH_SELF_REFERENCE_DECL | FileCheck --check-prefix=CHECK-SELF-REF-DECL %s
 //
 // CHECK-WITH-DECL-NOT: @llvm.memcpy
 // CHECK-NO-DECL: @llvm.memcpy
 // CHECK-SELF-REF-DECL-LABEL: define {{.*}}i8* @memcpy.inline
-// CHECK-SELF-REF-DECL:   @memcpy(
+// CHECK-SELF-REF-DECL:   @llvm.memcpy.{{.*}}(
 //
 #include 
 void test(void *dest, void const *from, size_t n) {
Index: clang/test/CodeGen/memcpy-inline-builtin.c
===
--- clang/test/CodeGen/memcpy-inline-builtin.c
+++ clang/test/CodeGen/memcpy-inline-builtin.c
@@ -32,13 +32,39 @@
 // CHECK-NEXT:store i8* [[TMP0]], i8** [[A_ADDR_I]], align 8
 // CHECK-NEXT:store i8* [[TMP1]], i8** [[B_ADDR_I]], align 8
 // CHECK-NEXT:store i64 [[TMP2]], i64* [[C_ADDR_I]], align 8
-// CHECK-NEXT:call void asm sideeffect "# memcpy.inline marker", "~{dirflag},~{fpsr},~{flags}"() #[[ATTR2:[0-9]+]], !srcloc !2
+// CHECK-NEXT:call void asm sideeffect "# memcpy.inline marker", "~{dirflag},~{fpsr},~{flags}"() #[[ATTR4:[0-9]+]], !srcloc !2
 // CHECK-NEXT:[[TMP3:%.*]] = load i8*, i8** [[A_ADDR_I]], align 8
 // CHECK-NEXT:[[TMP4:%.*]] = load i8*, i8** [[B_ADDR_I]], align 8
 // CHECK-NEXT:[[TMP5:%.*]] = load i64, i64* [[C_ADDR_I]], align 8
-// CHECK-NEXT:call void @llvm.memcpy.p0i8.p0i8.i64(i8* align 1 [[TMP3]], i8* align 1 [[TMP4]], i64 [[TMP5]], i1 false) #[[ATTR2]]
+// CHECK-NEXT:call void @llvm.memcpy.p0i8.p0i8.i64(i8* align 1 [[TMP3]], i8* align 1 [[TMP4]], i64 [[TMP5]], i1 false) #[[ATTR4]]
+// CHECK-NEXT:ret i8* [[TMP3]]
+//
+void *foo(void *a, const void *b, size_t c) {
+  return memcpy(a, b, c);
+}
+
+// CHECK-LABEL: @bar(
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:[[A_ADDR:%.*]] = alloca i8*, align 8
+// CHECK-NEXT:[[B_ADDR:%.*]] = alloca i8*, align 8
+// CHECK-NEXT:[[C_ADDR:%.*]] = alloca i64, align 8
+// CHECK-NEXT:[[CPY:%.*]] = alloca i8* (i8*, i8*, i64)*, align 8
+// CHECK-NEXT:store i8* [[A:%.*]], i8** [[A_ADDR]], align 8
+// CHECK-NEXT:store i8* [[B:%.*]], i8** [[B_ADDR]], align 8
+// CHECK-NEXT:store i64 [[C:%.*]], i64* [[C_ADDR]], align 8
+// CHECK-NEXT:[[TMP0:%.*]] = load i64, i64*

[PATCH] D95168: [clang-format] Add InsertBraces option

2021-10-03 Thread Owen Pan via Phabricator via cfe-commits 
owenpan added a comment.

In D95168#3038531 , @MyDeveloperDay 
wrote:

> @tiagoma are you still interested in pursuing this? I have some suggestions
>
> 1. I'd like to move the BraceInserter Into its own .cpp and .h files (like I 
> did with the QualifierAlignmentFixer)
> 2. I'd like to move the unit tests into their own .cpp file  (because I think 
> we need to actually unit tests their functions of BraceInserter more than 
> just testing if via verfiyFormat and I think its cleaner as FormatTest.cpp is 
> very large)
> 3. I'd like to see what it would take to remove braces, (eliding the braces 
> on small ifs and control statements is about the number one review comments 
> in LLVM)

Eliding braces would be much more complicated and should be tackled separately. 
Below are just some examples:

  void f() {
// Braces can be removed:
if (a) {
  b;
} else if (c) {
  d;
} else {
  e;
}
  
if (a) {
  b;
  c;
} else if (d) { // Braces may be removed depending on the style.
  #undef NDEBUG
  e;
} else if (g) {
  // comment
} else {
  {
h;
  }
}
  
if (a) {
  if (b) { // Braces can be removed.
c;
  }
} else if (d) { // Braces may be removed depending on the style.
  e;
}
  
if (a) { // Braces can be removed if the dangling-else warning can be 
ignored.
  if (b) { // Braces may be removed depending on the style.
c;
// comment
  } else if (d) { // Braces may be removed depending on the style.
e;
  }
}
  
// Braces can be removed:
if (a) {
  if (b) {
c;
  }
}
  
// Braces may be removed depending on the style:
if (a) {
  if (b) {
c;
  } else {
d;
  }
} else {
  e;
}
  
if (a) { // Braces may be removed depending on the style.
  // Braces can be removed:
  if (b) {
c;
  } else if (d) {
e;
  }
} else { // Braces may be removed depending on the style.
  g;
}
  
// Braces can be removed:
if (a) {
  b;
} else {
  if (c) {
d;
  } else {
e;
  }
}

I have an experimental implementation that reformats the above to the following 
(avoiding the dangling-else warning, matching braces of `if` and `else`, etc.):

  void f() {
// Braces can be removed:
if (a)
  b;
else if (c)
  d;
else
  e;
  
if (a) {
  b;
  c;
} else if (d) { // Braces may be removed depending on the style.
  #undef NDEBUG
  e;
} else if (g) {
  // comment
} else {
  { h; }
}
  
if (a) {
  if (b) // Braces can be removed.
c;
} else if (d) { // Braces may be removed depending on the style.
  e;
}
  
if (a) { // Braces can be removed if the dangling-else warning can be 
ignored.
  if (b) { // Braces may be removed depending on the style.
c;
// comment
  } else if (d) { // Braces may be removed depending on the style.
e;
  }
}
  
// Braces can be removed:
if (a)
  if (b)
c;
  
// Braces may be removed depending on the style:
if (a)
  if (b)
c;
  else
d;
else
  e;
  
if (a) { // Braces may be removed depending on the style.
  // Braces can be removed:
  if (b)
c;
  else if (d)
e;
} else { // Braces may be removed depending on the style.
  g;
}
  
// Braces can be removed:
if (a)
  b;
else if (c)
  d;
else
  e;


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95168

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

[PATCH] D111009: Update inline builtin handling to honor gnu inline attribute

2021-10-03 Thread Nick Desaulniers via Phabricator via cfe-commits 
nickdesaulniers added a comment.

With Diff 376790, I'm seeing a similar failure to 
https://github.com/ClangBuiltLinux/linux/issues/1466#issue-1011472964, but from 
different TUs.

  ld.lld: error: duplicate symbol: memcpy.inline
  >>> defined at file.c
  >>>fs/efivarfs/file.o:(memcpy.inline)
  >>> defined at super.c
  >>>fs/efivarfs/super.o:(.text+0x800)

let me see if I can put together another reproducer.


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

https://reviews.llvm.org/D111009

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

[PATCH] D111009: Update inline builtin handling to honor gnu inline attribute

2021-10-03 Thread serge via Phabricator via cfe-commits 
serge-sans-paille updated this revision to Diff 376790.
serge-sans-paille added a comment.

+ extra test case
+ avoid Playing with Twines
+ fix storage of external declaration of inline builtins
+ minor nits


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

https://reviews.llvm.org/D111009

Files:
  clang/lib/AST/Decl.cpp
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/test/CodeGen/memcmp-inline-builtin-to-asm.c
  clang/test/CodeGen/memcpy-inline-builtin.c
  clang/test/CodeGen/memcpy-nobuiltin.c
  clang/test/CodeGen/memcpy-nobuiltin.inc
  clang/test/CodeGen/pr9614.c

Index: clang/test/CodeGen/pr9614.c
===
--- clang/test/CodeGen/pr9614.c
+++ clang/test/CodeGen/pr9614.c
@@ -32,14 +32,14 @@
 
 // CHECK-LABEL: define{{.*}} void @f()
 // CHECK: call void @foo()
-// CHECK: call i32 @abs(i32 %0)
+// CHECK: call i32 @abs(i32 0)
 // CHECK: call i8* @strrchr(
 // CHECK: call void @llvm.prefetch.p0i8(
 // CHECK: call i8* @memchr(
 // CHECK: ret void
 
 // CHECK: declare void @foo()
+// CHECK: declare i32 @abs(i32
 // CHECK: declare i8* @strrchr(i8*, i32)
 // CHECK: declare i8* @memchr(
-// CHECK: declare i32 @abs(i32
 // CHECK: declare void @llvm.prefetch.p0i8(
Index: clang/test/CodeGen/memcpy-nobuiltin.inc
===
--- clang/test/CodeGen/memcpy-nobuiltin.inc
+++ clang/test/CodeGen/memcpy-nobuiltin.inc
@@ -2,7 +2,7 @@
 extern void *memcpy(void *dest, void const *from, size_t n);
 
 #ifdef WITH_DECL
-inline __attribute__((always_inline)) void *memcpy(void *dest, void const *from, size_t n) {
+inline __attribute__((always_inline)) __attribute__((gnu_inline)) void *memcpy(void *dest, void const *from, size_t n) {
   char const *ifrom = from;
   char *idest = dest;
   while (n--)
@@ -11,7 +11,7 @@
 }
 #endif
 #ifdef WITH_SELF_REFERENCE_DECL
-inline __attribute__((always_inline)) void *memcpy(void *dest, void const *from, size_t n) {
+inline __attribute__((always_inline)) __attribute__((gnu_inline)) void *memcpy(void *dest, void const *from, size_t n) {
   if (n != 0)
 memcpy(dest, from, n);
   return dest;
Index: clang/test/CodeGen/memcpy-nobuiltin.c
===
--- clang/test/CodeGen/memcpy-nobuiltin.c
+++ clang/test/CodeGen/memcpy-nobuiltin.c
@@ -5,7 +5,7 @@
 // CHECK-WITH-DECL-NOT: @llvm.memcpy
 // CHECK-NO-DECL: @llvm.memcpy
 // CHECK-SELF-REF-DECL-LABEL: define {{.*}}i8* @memcpy.inline
-// CHECK-SELF-REF-DECL:   @memcpy(
+// CHECK-SELF-REF-DECL:   @llvm.memcpy.{{.*}}(
 //
 #include 
 void test(void *dest, void const *from, size_t n) {
Index: clang/test/CodeGen/memcpy-inline-builtin.c
===
--- clang/test/CodeGen/memcpy-inline-builtin.c
+++ clang/test/CodeGen/memcpy-inline-builtin.c
@@ -32,13 +32,39 @@
 // CHECK-NEXT:store i8* [[TMP0]], i8** [[A_ADDR_I]], align 8
 // CHECK-NEXT:store i8* [[TMP1]], i8** [[B_ADDR_I]], align 8
 // CHECK-NEXT:store i64 [[TMP2]], i64* [[C_ADDR_I]], align 8
-// CHECK-NEXT:call void asm sideeffect "# memcpy.inline marker", "~{dirflag},~{fpsr},~{flags}"() #[[ATTR2:[0-9]+]], !srcloc !2
+// CHECK-NEXT:call void asm sideeffect "# memcpy.inline marker", "~{dirflag},~{fpsr},~{flags}"() #[[ATTR4:[0-9]+]], !srcloc !2
 // CHECK-NEXT:[[TMP3:%.*]] = load i8*, i8** [[A_ADDR_I]], align 8
 // CHECK-NEXT:[[TMP4:%.*]] = load i8*, i8** [[B_ADDR_I]], align 8
 // CHECK-NEXT:[[TMP5:%.*]] = load i64, i64* [[C_ADDR_I]], align 8
-// CHECK-NEXT:call void @llvm.memcpy.p0i8.p0i8.i64(i8* align 1 [[TMP3]], i8* align 1 [[TMP4]], i64 [[TMP5]], i1 false) #[[ATTR2]]
+// CHECK-NEXT:call void @llvm.memcpy.p0i8.p0i8.i64(i8* align 1 [[TMP3]], i8* align 1 [[TMP4]], i64 [[TMP5]], i1 false) #[[ATTR4]]
+// CHECK-NEXT:ret i8* [[TMP3]]
+//
+void *foo(void *a, const void *b, size_t c) {
+  return memcpy(a, b, c);
+}
+
+// CHECK-LABEL: @bar(
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:[[A_ADDR:%.*]] = alloca i8*, align 8
+// CHECK-NEXT:[[B_ADDR:%.*]] = alloca i8*, align 8
+// CHECK-NEXT:[[C_ADDR:%.*]] = alloca i64, align 8
+// CHECK-NEXT:[[CPY:%.*]] = alloca i8* (i8*, i8*, i64)*, align 8
+// CHECK-NEXT:store i8* [[A:%.*]], i8** [[A_ADDR]], align 8
+// CHECK-NEXT:store i8* [[B:%.*]], i8** [[B_ADDR]], align 8
+// CHECK-NEXT:store i64 [[C:%.*]], i64* [[C_ADDR]], align 8
+// CHECK-NEXT:[[TMP0:%.*]] = load i64, i64* [[C_ADDR]], align 8
+// CHECK-NEXT:[[CMP:%.*]] = icmp ugt i64 [[TMP0]], 10
+// CHECK-NEXT:[[TMP1:%.*]] = zext i1 [[CMP]] to i64
+// CHECK-NEXT:[[COND:%.*]] = select i1 [[CMP]], i8* (i8*, i8*, i64)* @memcpy, i8* (i8*, i8*, i64)* @foo
+// CHECK-NEXT:store i8* (i8*, i8*, i64)* [[COND]], i8* (i8*, i8*, i64)** [[CPY]], align 8
+// CHECK-NEXT:[[TMP2:%.*]] = load i8* (i8*, i8*, i64)*, i8* (i8*, i8*, i64)** [[CPY]], align 8
+// CHECK-NEXT:

[PATCH] D103938: Diagnose -Wunused-value based on CFG reachability

2021-10-03 Thread Yuanfang Chen via Phabricator via cfe-commits 
ychen added inline comments.



Comment at: clang/include/clang/Sema/Sema.h:5120
+  ///
+  /// \param Statement If Statement is non-null, delay reporting the
+  /// diagnostic until the function body is parsed, and then do a basic

xbolva00 wrote:
> Please adjust documentation, there is no "Statement" param.
Fixed in a944f801cacdaa40. Thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103938

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

[clang] a944f80 - [Clang][NFC] Fix the comment for Sema::DiagIfReachable

2021-10-03 Thread Yuanfang Chen via cfe-commits 

Author: Yuanfang Chen
Date: 2021-10-03T13:03:24-07:00
New Revision: a944f801cacdaa40b3869986844a6ffa08b87c19

URL: 
https://github.com/llvm/llvm-project/commit/a944f801cacdaa40b3869986844a6ffa08b87c19
DIFF: 
https://github.com/llvm/llvm-project/commit/a944f801cacdaa40b3869986844a6ffa08b87c19.diff

LOG: [Clang][NFC] Fix the comment for Sema::DiagIfReachable

Added: 


Modified: 
clang/include/clang/Sema/Sema.h

Removed: 




diff  --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 64863e3a5781..6c0f5cee7eaf 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -5114,13 +5114,13 @@ class Sema final {
   /// conversion.
   ExprResult tryConvertExprToType(Expr *E, QualType Ty);
 
-  /// Conditionally issue a diagnostic based on the statement's reachability
-  /// analysis evaluation context.
+  /// Conditionally issue a diagnostic based on the statements's reachability
+  /// analysis.
   ///
-  /// \param Statement If Statement is non-null, delay reporting the
-  /// diagnostic until the function body is parsed, and then do a basic
-  /// reachability analysis to determine if the statement is reachable.
-  /// If it is unreachable, the diagnostic will not be emitted.
+  /// \param Stmts If Stmts is non-empty, delay reporting the diagnostic until
+  /// the function body is parsed, and then do a basic reachability analysis to
+  /// determine if the statement is reachable. If it is unreachable, the
+  /// diagnostic will not be emitted.
   bool DiagIfReachable(SourceLocation Loc, ArrayRef Stmts,
const PartialDiagnostic );
 



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

[PATCH] D111009: Update inline builtin handling to honor gnu inline attribute

2021-10-03 Thread Nick Desaulniers via Phabricator via cfe-commits 
nickdesaulniers added a comment.

Here's a reduced test case from the kernel. Let's add a unit test for it.

  const void *con_unify_unimap_p1;
  extern inline __attribute__((__always_inline__)) __attribute__((gnu_inline))
  int memcmp(const void *p, const void *q, unsigned long size) {
__builtin_memcmp(p, q, size);
  }
  void con_unify_unimap_q1(void) {
memcmp(con_unify_unimap_p1, con_unify_unimap_q1, sizeof(int));
  }


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111009

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

[PATCH] D111009: Update inline builtin handling to honor gnu inline attribute

2021-10-03 Thread Nick Desaulniers via Phabricator via cfe-commits 
nickdesaulniers added inline comments.



Comment at: clang/lib/CodeGen/CGExpr.cpp:4911
+  if (!Clone) {
+Clone = llvm::Function::Create(Fn->getFunctionType(), Fn->getLinkage(),
+   Fn->getAddressSpace(),

given the reported ICE, I wonder if we shouldn't be reusing the `Fn`'s linkage, 
but explicitly setting extern? Same with below.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111009

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

[PATCH] D111009: Update inline builtin handling to honor gnu inline attribute

2021-10-03 Thread Nick Desaulniers via Phabricator via cfe-commits 
nickdesaulniers added a comment.

Note: kernel tests need to have `CONFIG_FORTIFY_SOURCE=y` enabled in the config 
to use the fortified routines; this isn't enabled by default in an x86_64 
`defconfig` build.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111009

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

[PATCH] D111009: Update inline builtin handling to honor gnu inline attribute

2021-10-03 Thread Nick Desaulniers via Phabricator via cfe-commits 
nickdesaulniers added a comment.

hmm...this seems to cause a bunch of ICEs building the kernel:

  Global is external, but doesn't have external or weak linkage!
  i64 (i8*)* @strlen
  fatal error: error in backend: Broken module found, compilation aborted!
  Global is external, but doesn't have external or weak linkage!
  i8* (i8*, i8*, i64)* @strncpy
  Global is external, but doesn't have external or weak linkage!
  i8* (i8*, i8*)* @strcpy
  Global is external, but doesn't have external or weak linkage!
  i64 (i8*)* @strlen
  Global is external, but doesn't have external or weak linkage!
  i64 (i8*, i8*, i64)* @strlcpy
  Global is external, but doesn't have external or weak linkage!
  i8* (i8*, i32, i64)* @memchr




Comment at: clang/lib/CodeGen/CGExpr.cpp:4895
+
+// Replaceable builtin provide their own implementation of a builtin. If we
+// are in an inline builtin implementation, avoid trivial infinite

s/Replaceable builtin/Replaceable builtins/



Comment at: clang/lib/CodeGen/CGExpr.cpp:4898
 // recursion.
+StringRef FDInlineName = (FD->getName() + ".inline").str();
 if (!FD->isInlineBuiltinDeclaration() ||

When building this patch (with Clang, via cmake vars `-DCMAKE_C_COMPILER=` and 
`-DCMAKE_CXX_COMPILER=`), I observe the following warning:

```
[99/110] Building CXX object 
tools/clang/lib/CodeGen/CMakeFiles/obj.clangCodeGen.dir/CGExpr.cpp.o
/android0/llvm-project/clang/lib/CodeGen/CGExpr.cpp:4898:30: warning: object 
backing the pointer will be destroyed at the end of the full-expression 
[-Wdangling-gsl]
StringRef FDInlineName = (FD->getName() + ".inline").str();
 ^
```

This leads to crashes when using the resulting image built with assertions 
enabled.

```
diff --git a/clang/lib/CodeGen/CGExpr.cpp b/clang/lib/CodeGen/CGExpr.cpp
index afed918c5c7f..cb6469d58b4c 100644
--- a/clang/lib/CodeGen/CGExpr.cpp
+++ b/clang/lib/CodeGen/CGExpr.cpp
@@ -4895,9 +4895,9 @@ static CGCallee EmitDirectCallee(CodeGenFunction , 
GlobalDecl GD) {
 // Replaceable builtin provide their own implementation of a builtin. If we
 // are in an inline builtin implementation, avoid trivial infinite
 // recursion.
-StringRef FDInlineName = (FD->getName() + ".inline").str();
+Twine FDInlineName = FD->getName() + ".inline";
 if (!FD->isInlineBuiltinDeclaration() ||
-CGF.CurFn->getName() == FDInlineName) {
+CGF.CurFn->getName() == FDInlineName.str()) {
   return CGCallee::forBuiltin(builtinID, FD);
 }
 // When directing calling an inline builtin, call it through it's mangled
@@ -4906,7 +4906,7 @@ static CGCallee EmitDirectCallee(CodeGenFunction , 
GlobalDecl GD) {
   llvm::Constant *CalleePtr = EmitFunctionDeclPointer(CGF.CGM, GD);
   llvm::Function *Fn = llvm::cast(CalleePtr);
   llvm::Module  = CGF.CGM.getModule();
-  llvm::Function *Clone = M.getFunction(FDInlineName);
+  llvm::Function *Clone = M.getFunction(FDInlineName.str());
   if (!Clone) {
 Clone = llvm::Function::Create(Fn->getFunctionType(), Fn->getLinkage(),
Fn->getAddressSpace(),
```



Comment at: clang/lib/CodeGen/CGExpr.cpp:4901
+CGF.CurFn->getName() == FDInlineName) {
   return CGCallee::forBuiltin(builtinID, FD);
+}

I think if you flip the order of the branches by inverting the logic in the 
conditional, then you could avoid needing braces for the single statement 
branch. ie.

```
if (FD->isInlineBuiltinDeclaration() && CGF.CurFn->getName() != FDInlineName) {
  llvm::Constant *CalleePtr = EmitFunctionDeclPointer(CGF.CGM, GD);
  ...
} else
  return CGCallee::forBuiltin(builtinID, FD);
```



Comment at: clang/lib/CodeGen/CGExpr.cpp:4908
+  llvm::Function *Fn = llvm::cast(CalleePtr);
+  llvm::Module  = CGF.CGM.getModule();
+  llvm::Function *Clone = M.getFunction(FDInlineName);

If you have a handle to a `Function`, you can get a pointer to it's module via 
`Function::getParent()`.



Comment at: clang/lib/CodeGen/CGExpr.cpp:4913
+   Fn->getAddressSpace(),
+   FD->getName() + ".inline", );
+Clone->addFnAttr(llvm::Attribute::AlwaysInline);

Can we reuse `FDInlineName` here for the 4th param?



Comment at: clang/lib/CodeGen/CodeGenFunction.cpp:1302
+  if (FD->isInlineBuiltinDeclaration() && Fn) {
+llvm::Module  = CGM.getModule();
+llvm::Function *Clone = M.getFunction((Fn->getName() + ".inline").str());

could use `Fn->getParent()` here, too.



Comment at: clang/lib/CodeGen/CodeGenFunction.cpp:1307
+ Fn->getAddressSpace(),
+ (Fn->getName() + ".inline").str(),

[PATCH] D110668: [clang-cl] Accept `#pragma warning(disable : N)` for some N

2021-10-03 Thread Dávid Bolvanský via Phabricator via cfe-commits 
xbolva00 added a subscriber: aaron.ballman.
xbolva00 added a comment.

In D110668#3036361 , @thakis wrote:

> In D110668#3034576 , @xbolva00 
> wrote:
>
>> Please next time give a bit more time to potential reviewers / other folks 
>> outside your org. The whole lifecycle of this patch (posted - landed) took < 
>> 24h.
>
> Is there anything wrong with the patch?
>
> I agree that it's good to let larger changes sit for a bit, but this seems 
> like a fairly small and inconsequential change to me. Many patches land with 
> a review time < 24h.
>
> In any case, happy to address post-commit review comments too of course.

I think I would prefer to implement such "mapping" in DiagnosticGroups.td 
instead of current solution. cc @aaron.ballman as well, as he is an exprienced 
reviewer here.

What I mean,  for example:

  def UnusedParameter : DiagGroup<"unused-parameter", 4100>;


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110668

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

[PATCH] D110216: [clang] retain type sugar in auto / template argument deduction

2021-10-03 Thread Vassil Vassilev via Phabricator via cfe-commits 
v.g.vassilev added a comment.

In D110216#3038826 , @mizvekov wrote:

> In D110216#3038797 , @v.g.vassilev 
> wrote:
>
>> Over the years we had some interest from people but never actually got 
>> implemented. Here 
>>  were some 
>> ideas @rsmith and I discussed over the years. If that is helpful, let me 
>> know if I should dig a bit more into private email exchange.
>
> Sure, that is helpful :)
>
> There is other lower hanging fruit where we are losing sugar, and it would be 
> a shame if we implemented this but then did not get much benefit from it 
> because the sugar never got into the template argument in the first place.
>
> One such example is that we do not mark as 'elaborate' types which are 
> written bare, without any scope specifiers.
>
> So for example in a case like this:
>
>   namespace foo {
> struct Foo {};
> Foo x = 0;
>   };
>
> We would still diagnose that assignment with the type of the variable printed 
> as 'foo::Foo' instead of just 'Foo', as it was written, because the parser 
> will have produced a type that is not wrapped in an ElaboratedType (or 
> perhaps some other cheaper mechanism).

Handling that case is nice. I am more interested in retaining the sugar in 
template instantiations as it is essential for an optimization in our I/O 
system which uses clang as a library. If implement the instantiation diagnostic 
we can get rid of several hacky patches: 
https://github.com/vgvassilev/clang/commit/d87e2bbc8a266e295ee5a2065f1e587b325d4284
 
https://github.com/vgvassilev/clang/commit/fcc492fcab14e8b8dc156688dda6f237a04563a7
 and 
https://github.com/vgvassilev/clang/commit/fe17b953325530267643f3391bfd59ac1519ef39




Comment at: clang/lib/Sema/SemaTemplateDeduction.cpp:1414
   TDF &= ~TDF_TopLevelParameterTypeList;
-  if (isForwardingReference(Param, 0) && Arg->isLValueReferenceType())
-Param = Param->getPointeeType();
+  if (isForwardingReference(P, 0) && A->isLValueReferenceType())
+P = P->getPointeeType();





Comment at: clang/lib/Sema/SemaTemplateDeduction.cpp:1777
   S, TemplateParams, NTTP, Noexcept, S.Context.BoolTy,
-  /*ArrayBound*/true, Info, Deduced);
+  /*ArrayBound*/ true, Info, Deduced);
 




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110216

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

[PATCH] D110745: Redefine deref(N) attribute family as point-in-time semantics (aka deref-at-point)

2021-10-03 Thread Dávid Bolvanský via Phabricator via cfe-commits 
xbolva00 added a comment.

This really needs to be properly benchmarked.




Comment at: llvm/test/Transforms/InstCombine/AMDGPU/memcpy-from-constant.ll:9
 
 ; Simple memcpy to alloca from constant address space argument.
 define i8 @memcpy_constant_arg_ptr_to_alloca([32 x i8] addrspace(4)* noalias 
readonly align 4 dereferenceable(32) %arg, i32 %idx) {

No longer happens..



Comment at: llvm/test/Transforms/LICM/hoist-deref-load.ll:419
 ; and we want to hoist the load of %c out of the loop. This can be done only
 ; because the dereferenceable meatdata on the c = *cptr load.
 define void @test7(i32* noalias %a, i32* %b, i32** %cptr, i32 %n) #0 {

Regression in that C code?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110745

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

[PATCH] D95168: [clang-format] Add InsertBraces option

2021-10-03 Thread MyDeveloperDay via Phabricator via cfe-commits 
MyDeveloperDay requested changes to this revision.
MyDeveloperDay added a comment.
This revision now requires changes to proceed.

This patch needs rebasing to main


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95168

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

[PATCH] D110216: [clang] retain type sugar in auto / template argument deduction

2021-10-03 Thread Matheus Izvekov via Phabricator via cfe-commits 
mizvekov added a comment.

In D110216#3038797 , @v.g.vassilev 
wrote:

> Over the years we had some interest from people but never actually got 
> implemented. Here 
>  were some 
> ideas @rsmith and I discussed over the years. If that is helpful, let me know 
> if I should dig a bit more into private email exchange.

Sure, that is helpful :)

There is other lower hanging fruit where we are losing sugar, and it would be a 
shame if we implemented this but then did not get much benefit from it because 
the sugar never got into the template argument in the first place.

One such example is that we do not mark as 'elaborate' types which are written 
bare, without any scope specifiers.

So for example in a case like this:

  namespace foo {
struct Foo {};
Foo x = 0;
  };

We would still diagnose that assignment with the type of the variable printed 
as 'foo::Foo' instead of just 'Foo', as it was written, because the parser will 
have produced a type that is not wrapped in an ElaboratedType (or perhaps some 
other cheaper mechanism).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110216

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

[PATCH] D110216: [clang] retain type sugar in auto / template argument deduction

2021-10-03 Thread Vassil Vassilev via Phabricator via cfe-commits 
v.g.vassilev added a comment.

In D110216#3035041 , @mizvekov wrote:

> In D110216#3032626 , @v.g.vassilev 
> wrote:
>
>> Thanks for working on this!How hard would it be to support:
>>
>>   using size_t = __SIZE_TYPE__;
>>   template struct Id { typedef T type; };
>>   int main() {
>> struct S {} s;
>> Id::type f = s; // just 'unsigned long', 'size_t' sugar has been 
>> lost
>>   }
>
> Actually supporting that is in my radar :)

Over the years we had some interest from people but never actually got 
implemented. Here 
 were some 
ideas @rsmith and I discussed over the years. If that is helpful, let me know 
if I should dig a bit more into private email exchange.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110216

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

[clang] fb84aa2 - Fixed warnings in target/parser codes produced by -Wbitwise-instead-of-logicala

2021-10-03 Thread Dávid Bolvanský via cfe-commits 

Author: Dávid Bolvanský
Date: 2021-10-03T15:04:01+02:00
New Revision: fb84aa2a8f52272cd0cb9510bac5404a3d4ec565

URL: 
https://github.com/llvm/llvm-project/commit/fb84aa2a8f52272cd0cb9510bac5404a3d4ec565
DIFF: 
https://github.com/llvm/llvm-project/commit/fb84aa2a8f52272cd0cb9510bac5404a3d4ec565.diff

LOG: Fixed warnings in target/parser codes produced by 
-Wbitwise-instead-of-logicala

Added: 


Modified: 
clang/lib/Lex/PPExpressions.cpp
llvm/lib/AsmParser/LLParser.cpp
llvm/lib/Target/AMDGPU/AMDGPUInstructionSelector.cpp
llvm/lib/Target/Lanai/LanaiAluCode.h
llvm/lib/Target/Mips/MipsSubtarget.cpp

Removed: 




diff  --git a/clang/lib/Lex/PPExpressions.cpp b/clang/lib/Lex/PPExpressions.cpp
index 1ebfae606a588..424cccfdb9eef 100644
--- a/clang/lib/Lex/PPExpressions.cpp
+++ b/clang/lib/Lex/PPExpressions.cpp
@@ -662,7 +662,7 @@ static bool EvaluateDirectiveSubExpr(PPValue , unsigned 
MinPrec,
 case tok::ampamp: // Logical && does not do UACs.
   break;  // No UAC
 default:
-  Res.setIsUnsigned(LHS.isUnsigned()|RHS.isUnsigned());
+  Res.setIsUnsigned(LHS.isUnsigned() || RHS.isUnsigned());
   // If this just promoted something from signed to unsigned, and if the
   // value was negative, warn about it.
   if (ValueLive && Res.isUnsigned()) {
@@ -822,7 +822,7 @@ static bool EvaluateDirectiveSubExpr(PPValue , unsigned 
MinPrec,
 
   // Usual arithmetic conversions (C99 6.3.1.8p1): result is unsigned if
   // either operand is unsigned.
-  Res.setIsUnsigned(RHS.isUnsigned() | AfterColonVal.isUnsigned());
+  Res.setIsUnsigned(RHS.isUnsigned() || AfterColonVal.isUnsigned());
 
   // Figure out the precedence of the token after the : part.
   PeekPrec = getPrecedence(PeekTok.getKind());

diff  --git a/llvm/lib/AsmParser/LLParser.cpp b/llvm/lib/AsmParser/LLParser.cpp
index 75dcc66af2b8a..ebca7f3083810 100644
--- a/llvm/lib/AsmParser/LLParser.cpp
+++ b/llvm/lib/AsmParser/LLParser.cpp
@@ -8529,8 +8529,8 @@ bool 
LLParser::parseOptionalFFlags(FunctionSummary::FFlags ) {
   assert(Lex.getKind() == lltok::kw_funcFlags);
   Lex.Lex();
 
-  if ((int)parseToken(lltok::colon, "expected ':' in funcFlags") |
-  (int)parseToken(lltok::lparen, "expected '(' in funcFlags"))
+  if (parseToken(lltok::colon, "expected ':' in funcFlags") ||
+  parseToken(lltok::lparen, "expected '(' in funcFlags"))
 return true;
 
   do {
@@ -8609,7 +8609,7 @@ bool 
LLParser::parseOptionalCalls(std::vector ) {
   assert(Lex.getKind() == lltok::kw_calls);
   Lex.Lex();
 
-  if (parseToken(lltok::colon, "expected ':' in calls") |
+  if (parseToken(lltok::colon, "expected ':' in calls") ||
   parseToken(lltok::lparen, "expected '(' in calls"))
 return true;
 
@@ -8701,8 +8701,8 @@ bool LLParser::parseOptionalVTableFuncs(VTableFuncList 
) {
   assert(Lex.getKind() == lltok::kw_vTableFuncs);
   Lex.Lex();
 
-  if ((int)parseToken(lltok::colon, "expected ':' in vTableFuncs") |
-  (int)parseToken(lltok::lparen, "expected '(' in vTableFuncs"))
+  if (parseToken(lltok::colon, "expected ':' in vTableFuncs") ||
+  parseToken(lltok::lparen, "expected '(' in vTableFuncs"))
 return true;
 
   IdToIndexMapType IdToIndexMap;

diff  --git a/llvm/lib/Target/AMDGPU/AMDGPUInstructionSelector.cpp 
b/llvm/lib/Target/AMDGPU/AMDGPUInstructionSelector.cpp
index 3c4191a485603..50e515d6c4fe1 100644
--- a/llvm/lib/Target/AMDGPU/AMDGPUInstructionSelector.cpp
+++ b/llvm/lib/Target/AMDGPU/AMDGPUInstructionSelector.cpp
@@ -1826,8 +1826,9 @@ bool 
AMDGPUInstructionSelector::selectG_SELECT(MachineInstr ) const {
 .add(I.getOperand(2))
 .add(I.getOperand(3));
 
-bool Ret = constrainSelectedInstRegOperands(*Select, TII, TRI, RBI) |
-   constrainSelectedInstRegOperands(*CopySCC, TII, TRI, RBI);
+bool Ret = false;
+Ret |= constrainSelectedInstRegOperands(*Select, TII, TRI, RBI);
+Ret |= constrainSelectedInstRegOperands(*CopySCC, TII, TRI, RBI);
 I.eraseFromParent();
 return Ret;
   }

diff  --git a/llvm/lib/Target/Lanai/LanaiAluCode.h 
b/llvm/lib/Target/Lanai/LanaiAluCode.h
index 728332bff00b8..69be055427231 100644
--- a/llvm/lib/Target/Lanai/LanaiAluCode.h
+++ b/llvm/lib/Target/Lanai/LanaiAluCode.h
@@ -70,7 +70,7 @@ inline static unsigned makePostOp(unsigned AluOp) {
 }
 
 inline static bool modifiesOp(unsigned AluOp) {
-  return isPreOp(AluOp) | isPostOp(AluOp);
+  return isPreOp(AluOp) || isPostOp(AluOp);
 }
 
 inline static const char *lanaiAluCodeToString(unsigned AluOp) {

diff  --git a/llvm/lib/Target/Mips/MipsSubtarget.cpp 
b/llvm/lib/Target/Mips/MipsSubtarget.cpp
index 8bb9d75e9173a..0ae2d584a1a6c 100644
--- a/llvm/lib/Target/Mips/MipsSubtarget.cpp
+++ b/llvm/lib/Target/Mips/MipsSubtarget.cpp
@@ -78,7 +78,7 @@ MipsSubtarget::MipsSubtarget(const Triple , StringRef CPU, 
StringRef FS,

[PATCH] D103938: Diagnose -Wunused-value based on CFG reachability

2021-10-03 Thread Dávid Bolvanský via Phabricator via cfe-commits 
xbolva00 added inline comments.



Comment at: clang/include/clang/Sema/Sema.h:5120
+  ///
+  /// \param Statement If Statement is non-null, delay reporting the
+  /// diagnostic until the function body is parsed, and then do a basic

Please adjust documentation, there is no "Statement" param.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103938

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

[PATCH] D111009: Update inline builtin handling to honor gnu inline attribute

2021-10-03 Thread serge via Phabricator via cfe-commits 
serge-sans-paille created this revision.
serge-sans-paille added reviewers: kees, nickdesaulniers.
serge-sans-paille requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Per the GCC info page:

  If the function is declared 'extern', then this definition of the
  function is used only for inlining.  In no case is the function
  compiled as a standalone function, not even if you take its address
  explicitly.  Such an address becomes an external reference, as if
  you had only declared the function, and had not defined it.

Respect that behavior for inline builtins: keep the original definition, and
generate a copy of the declaration suffixed by '.inline' that's only referenced
in direct call.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D111009

Files:
  clang/lib/AST/Decl.cpp
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/test/CodeGen/memcpy-inline-builtin.c
  clang/test/CodeGen/memcpy-nobuiltin.c
  clang/test/CodeGen/memcpy-nobuiltin.inc
  clang/test/CodeGen/pr9614.c

Index: clang/test/CodeGen/pr9614.c
===
--- clang/test/CodeGen/pr9614.c
+++ clang/test/CodeGen/pr9614.c
@@ -32,14 +32,14 @@
 
 // CHECK-LABEL: define{{.*}} void @f()
 // CHECK: call void @foo()
-// CHECK: call i32 @abs(i32 %0)
+// CHECK: call i32 @abs(i32 0)
 // CHECK: call i8* @strrchr(
 // CHECK: call void @llvm.prefetch.p0i8(
 // CHECK: call i8* @memchr(
 // CHECK: ret void
 
 // CHECK: declare void @foo()
+// CHECK: declare i32 @abs(i32
 // CHECK: declare i8* @strrchr(i8*, i32)
 // CHECK: declare i8* @memchr(
-// CHECK: declare i32 @abs(i32
 // CHECK: declare void @llvm.prefetch.p0i8(
Index: clang/test/CodeGen/memcpy-nobuiltin.inc
===
--- clang/test/CodeGen/memcpy-nobuiltin.inc
+++ clang/test/CodeGen/memcpy-nobuiltin.inc
@@ -2,7 +2,7 @@
 extern void *memcpy(void *dest, void const *from, size_t n);
 
 #ifdef WITH_DECL
-inline __attribute__((always_inline)) void *memcpy(void *dest, void const *from, size_t n) {
+inline __attribute__((always_inline)) __attribute__((gnu_inline)) void *memcpy(void *dest, void const *from, size_t n) {
   char const *ifrom = from;
   char *idest = dest;
   while (n--)
@@ -11,7 +11,7 @@
 }
 #endif
 #ifdef WITH_SELF_REFERENCE_DECL
-inline __attribute__((always_inline)) void *memcpy(void *dest, void const *from, size_t n) {
+inline __attribute__((always_inline)) __attribute__((gnu_inline)) void *memcpy(void *dest, void const *from, size_t n) {
   if (n != 0)
 memcpy(dest, from, n);
   return dest;
Index: clang/test/CodeGen/memcpy-nobuiltin.c
===
--- clang/test/CodeGen/memcpy-nobuiltin.c
+++ clang/test/CodeGen/memcpy-nobuiltin.c
@@ -5,7 +5,7 @@
 // CHECK-WITH-DECL-NOT: @llvm.memcpy
 // CHECK-NO-DECL: @llvm.memcpy
 // CHECK-SELF-REF-DECL-LABEL: define {{.*}}i8* @memcpy.inline
-// CHECK-SELF-REF-DECL:   @memcpy(
+// CHECK-SELF-REF-DECL:   @llvm.memcpy.{{.*}}(
 //
 #include 
 void test(void *dest, void const *from, size_t n) {
Index: clang/test/CodeGen/memcpy-inline-builtin.c
===
--- clang/test/CodeGen/memcpy-inline-builtin.c
+++ clang/test/CodeGen/memcpy-inline-builtin.c
@@ -32,13 +32,39 @@
 // CHECK-NEXT:store i8* [[TMP0]], i8** [[A_ADDR_I]], align 8
 // CHECK-NEXT:store i8* [[TMP1]], i8** [[B_ADDR_I]], align 8
 // CHECK-NEXT:store i64 [[TMP2]], i64* [[C_ADDR_I]], align 8
-// CHECK-NEXT:call void asm sideeffect "# memcpy.inline marker", "~{dirflag},~{fpsr},~{flags}"() #[[ATTR2:[0-9]+]], !srcloc !2
+// CHECK-NEXT:call void asm sideeffect "# memcpy.inline marker", "~{dirflag},~{fpsr},~{flags}"() #[[ATTR4:[0-9]+]], !srcloc !2
 // CHECK-NEXT:[[TMP3:%.*]] = load i8*, i8** [[A_ADDR_I]], align 8
 // CHECK-NEXT:[[TMP4:%.*]] = load i8*, i8** [[B_ADDR_I]], align 8
 // CHECK-NEXT:[[TMP5:%.*]] = load i64, i64* [[C_ADDR_I]], align 8
-// CHECK-NEXT:call void @llvm.memcpy.p0i8.p0i8.i64(i8* align 1 [[TMP3]], i8* align 1 [[TMP4]], i64 [[TMP5]], i1 false) #[[ATTR2]]
+// CHECK-NEXT:call void @llvm.memcpy.p0i8.p0i8.i64(i8* align 1 [[TMP3]], i8* align 1 [[TMP4]], i64 [[TMP5]], i1 false) #[[ATTR4]]
+// CHECK-NEXT:ret i8* [[TMP3]]
+//
+void *foo(void *a, const void *b, size_t c) {
+  return memcpy(a, b, c);
+}
+
+// CHECK-LABEL: @bar(
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:[[A_ADDR:%.*]] = alloca i8*, align 8
+// CHECK-NEXT:[[B_ADDR:%.*]] = alloca i8*, align 8
+// CHECK-NEXT:[[C_ADDR:%.*]] = alloca i64, align 8
+// CHECK-NEXT:[[CPY:%.*]] = alloca i8* (i8*, i8*, i64)*, align 8
+// CHECK-NEXT:store i8* [[A:%.*]], i8** [[A_ADDR]], align 8
+// CHECK-NEXT:store i8* [[B:%.*]], i8** [[B_ADDR]], align 8
+// CHECK-NEXT:store i64 [[C:%.*]], i64* [[C_ADDR]], align 8
+// CHECK-NEXT:[[TMP0:%.*]] = load i64,

[PATCH] D95168: [clang-format] Add InsertBraces option

2021-10-03 Thread MyDeveloperDay via Phabricator via cfe-commits 
MyDeveloperDay added inline comments.



Comment at: clang/lib/Format/Format.cpp:2844
+
+if (Style.InsertBraces != FormatStyle::BIS_Never)
+  Passes.emplace_back([&](const Environment ) {

Why is this just C++ (LK_Cpp is also Objective C++ and C and Objective C I 
think)

Why wouldn't I want to add "{}" in C# and/or JavaScript?



Comment at: clang/unittests/Format/FormatTest.cpp:76
+// clang::format::internal::reformat does not run any of the options that
+// modify code for ObjC
+if (Style.Language == FormatStyle::LK_Cpp &&

Something here doesn't seem correct, I don't really understand the comment?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95168

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

[PATCH] D95168: [clang-format] Add InsertBraces option

2021-10-03 Thread MyDeveloperDay via Phabricator via cfe-commits 
MyDeveloperDay added inline comments.



Comment at: clang/include/clang/Format/Format.h:908
+  /// If set to ``BIS_Always`` will always insert braces.
+  /// The default is the disabled value of ``BIS_Never``.
+  BraceInsertionStyle InsertBraces;

Please add something like this
```
  /// \warning
  ///  Setting ``InsertBraces``  to something other than `Never`, COULD
  ///  lead to incorrect code formatting due to incorrect decisions made due to
  ///  clang-formats lack of full semantic information.
  ///  As such extra care should be taken to review code changes made by the use
  ///  of this option.
  /// \endwarning
  /// \version 14
```




Comment at: clang/lib/Format/Format.cpp:188
+IO.enumCase(Value, "Always", FormatStyle::BIS_Always);
+IO.enumCase(Value, "WrapLikely", FormatStyle::BIS_WrapLikely);
+  }

I'd like to see "Remove"


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95168

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

[PATCH] D95168: [clang-format] Add InsertBraces option

2021-10-03 Thread MyDeveloperDay via Phabricator via cfe-commits 
MyDeveloperDay added a comment.

@tiagoma are you still interested in pursuing this? I have some suggestions

1. I'd like to move the BraceInserter Into its own .cpp and .h files (like I 
did with the QualifierAlignmentFixer)
2. I'd like to move the unit tests into their own .cpp file  (because I think 
we need to actually unit tests their functions of BraceInserter more than just 
testing if via verfiyFormat and I think its cleaner as FormatTest.cpp is very 
large)
3. I'd like to see what it would take to remove braces, (eliding the braces on 
small ifs and control statements is about the number one review comments in 
LLVM)

I feel like we've general community agreement that clang-format can modify the 
tokens (following the RFC around QualifierAlignmentFixer) as long as the 
documentation carries a clear warning and its off by default.

If you are no longer interested in landing this, I am happy to consider taking 
it on.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95168

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

[clang] a76355d - Unbreak hexagon-check-builtins.c due to rGb1fcca388441

2021-10-03 Thread Dávid Bolvanský via cfe-commits 

Author: Dávid Bolvanský
Date: 2021-10-03T13:19:34+02:00
New Revision: a76355d570a96ebcb4e790bc06020f184351500d

URL: 
https://github.com/llvm/llvm-project/commit/a76355d570a96ebcb4e790bc06020f184351500d
DIFF: 
https://github.com/llvm/llvm-project/commit/a76355d570a96ebcb4e790bc06020f184351500d.diff

LOG: Unbreak hexagon-check-builtins.c due to rGb1fcca388441

Added: 


Modified: 
clang/lib/Sema/SemaChecking.cpp

Removed: 




diff  --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index bdd9fb495da2..66450f61c091 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -2988,8 +2988,8 @@ bool Sema::CheckHexagonBuiltinArgument(unsigned 
BuiltinID, CallExpr *TheCall) {
   unsigned M = 1 << A.Align;
   Min *= M;
   Max *= M;
-  Error |= SemaBuiltinConstantArgRange(TheCall, A.OpNum, Min, Max) ||
-   SemaBuiltinConstantArgMultiple(TheCall, A.OpNum, M);
+  Error |= SemaBuiltinConstantArgRange(TheCall, A.OpNum, Min, Max);
+  Error |= SemaBuiltinConstantArgMultiple(TheCall, A.OpNum, M);
 }
   }
   return Error;



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

[clang] f59cc95 - Reland "[Clang] Extend -Wbool-operation to warn about bitwise and of bools with side effects"

2021-10-03 Thread Dávid Bolvanský via cfe-commits 

Author: Dávid Bolvanský
Date: 2021-10-03T13:05:09+02:00
New Revision: f59cc9542bfb461d16ad12b2cc4be4abbfd9d96e

URL: 
https://github.com/llvm/llvm-project/commit/f59cc9542bfb461d16ad12b2cc4be4abbfd9d96e
DIFF: 
https://github.com/llvm/llvm-project/commit/f59cc9542bfb461d16ad12b2cc4be4abbfd9d96e.diff

LOG: Reland "[Clang] Extend -Wbool-operation to warn about bitwise and of bools 
with side effects"

This reverts commit a4933f57f3f0a45e1db1075f7285f0761a80fc06. New warnings were 
fixed.

Added: 
clang/test/Sema/warn-bitwise-and-bool.c
clang/test/Sema/warn-bitwise-or-bool.c

Modified: 
clang/include/clang/Basic/DiagnosticGroups.td
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/lib/Sema/SemaChecking.cpp
clang/test/Misc/warning-wall.c

Removed: 




diff  --git a/clang/include/clang/Basic/DiagnosticGroups.td 
b/clang/include/clang/Basic/DiagnosticGroups.td
index 761b323d0616..d9db3482dbda 100644
--- a/clang/include/clang/Basic/DiagnosticGroups.td
+++ b/clang/include/clang/Basic/DiagnosticGroups.td
@@ -64,7 +64,8 @@ def StringConversion : DiagGroup<"string-conversion">;
 def SignConversion : DiagGroup<"sign-conversion">;
 def PointerBoolConversion : DiagGroup<"pointer-bool-conversion">;
 def UndefinedBoolConversion : DiagGroup<"undefined-bool-conversion">;
-def BoolOperation : DiagGroup<"bool-operation">;
+def BitwiseInsteadOfLogical : DiagGroup<"bitwise-instead-of-logical">;
+def BoolOperation : DiagGroup<"bool-operation", [BitwiseInsteadOfLogical]>;
 def BoolConversion : DiagGroup<"bool-conversion", [PointerBoolConversion,
UndefinedBoolConversion]>;
 def IntConversion : DiagGroup<"int-conversion">;

diff  --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td 
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 1d4ea92c6520..c71a00b18432 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -66,6 +66,7 @@ def warn_infinite_recursive_function : Warning<
 def warn_comma_operator : Warning<"possible misuse of comma operator here">,
   InGroup>, DefaultIgnore;
 def note_cast_to_void : Note<"cast expression to void to silence warning">;
+def note_cast_operand_to_int : Note<"cast one or both operands to int to 
silence this warning">;
 
 // Constant expressions
 def err_expr_not_ice : Error<
@@ -7423,6 +7424,9 @@ def note_member_declared_here : Note<
   "member %0 declared here">;
 def note_member_first_declared_here : Note<
   "member %0 first declared here">;
+def warn_bitwise_instead_of_logical : Warning<
+  "use of bitwise '%0' with boolean operands">,
+  InGroup, DefaultIgnore;
 def warn_bitwise_negation_bool : Warning<
   "bitwise negation of a boolean expression%select{;| always evaluates to 
'true';}0 "
   "did you mean logical negation?">,

diff  --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index 615ba1aef8e6..bdd9fb495da2 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -13249,6 +13249,20 @@ static void AnalyzeImplicitConversions(
   << OrigE->getSourceRange() << T->isBooleanType()
   << FixItHint::CreateReplacement(UO->getBeginLoc(), "!");
 
+  if (const auto *BO = dyn_cast(SourceExpr))
+if ((BO->getOpcode() == BO_And || BO->getOpcode() == BO_Or) &&
+BO->getLHS()->isKnownToHaveBooleanValue() &&
+BO->getRHS()->isKnownToHaveBooleanValue() &&
+BO->getLHS()->HasSideEffects(S.Context) &&
+BO->getRHS()->HasSideEffects(S.Context)) {
+  S.Diag(BO->getBeginLoc(), diag::warn_bitwise_instead_of_logical)
+  << (BO->getOpcode() == BO_And ? "&" : "|") << OrigE->getSourceRange()
+  << FixItHint::CreateReplacement(
+ BO->getOperatorLoc(),
+ (BO->getOpcode() == BO_And ? "&&" : "||"));
+  S.Diag(BO->getBeginLoc(), diag::note_cast_operand_to_int);
+}
+
   // For conditional operators, we analyze the arguments as if they
   // were being fed directly into the output.
   if (auto *CO = dyn_cast(SourceExpr)) {

diff  --git a/clang/test/Misc/warning-wall.c b/clang/test/Misc/warning-wall.c
index a3686fb96a4c..a4a79bec934a 100644
--- a/clang/test/Misc/warning-wall.c
+++ b/clang/test/Misc/warning-wall.c
@@ -4,6 +4,7 @@ RUN: FileCheck --input-file=%t %s
  CHECK:-Wall
 CHECK-NEXT:  -Wmost
 CHECK-NEXT:-Wbool-operation
+CHECK-NEXT:-Wbitwise-instead-of-logical
 CHECK-NEXT:-Wchar-subscripts
 CHECK-NEXT:-Wcomment
 CHECK-NEXT:-Wdelete-non-virtual-dtor

diff  --git a/clang/test/Sema/warn-bitwise-and-bool.c 
b/clang/test/Sema/warn-bitwise-and-bool.c
new file mode 100644
index ..6bec1be1abde
--- /dev/null
+++ b/clang/test/Sema/warn-bitwise-and-bool.c
@@ -0,0 +1,63 @@
+// RUN: %clang_cc1 -x c -fsyntax-only -verify -Wbool-operation %s
+// RUN: %clang_cc1 -x c -fsyntax-only -verify -Wall %s
+//

[clang] b1fcca3 - Fixed warnings in LLVM produced by -Wbitwise-instead-of-logical

2021-10-03 Thread Dávid Bolvanský via cfe-commits 

Author: Dávid Bolvanský
Date: 2021-10-03T13:04:18+02:00
New Revision: b1fcca38844138d1950e1b34eb2be65b3bfc7352

URL: 
https://github.com/llvm/llvm-project/commit/b1fcca38844138d1950e1b34eb2be65b3bfc7352
DIFF: 
https://github.com/llvm/llvm-project/commit/b1fcca38844138d1950e1b34eb2be65b3bfc7352.diff

LOG: Fixed warnings in LLVM produced by -Wbitwise-instead-of-logical

Added: 


Modified: 
clang/lib/AST/Type.cpp
clang/lib/CodeGen/CGExpr.cpp
clang/lib/Sema/SemaChecking.cpp
clang/lib/Sema/SemaExpr.cpp
llvm/unittests/Support/TargetParserTest.cpp
llvm/utils/TableGen/CodeGenDAGPatterns.cpp

Removed: 




diff  --git a/clang/lib/AST/Type.cpp b/clang/lib/AST/Type.cpp
index 112218d6eb36..de57b40b221a 100644
--- a/clang/lib/AST/Type.cpp
+++ b/clang/lib/AST/Type.cpp
@@ -3759,8 +3759,8 @@ class CachedProperties {
 
   friend CachedProperties merge(CachedProperties L, CachedProperties R) {
 Linkage MergedLinkage = minLinkage(L.L, R.L);
-return CachedProperties(MergedLinkage,
- L.hasLocalOrUnnamedType() | 
R.hasLocalOrUnnamedType());
+return CachedProperties(MergedLinkage, L.hasLocalOrUnnamedType() ||
+   R.hasLocalOrUnnamedType());
   }
 };
 

diff  --git a/clang/lib/CodeGen/CGExpr.cpp b/clang/lib/CodeGen/CGExpr.cpp
index 5c92e1e7e073..a62a4d60830d 100644
--- a/clang/lib/CodeGen/CGExpr.cpp
+++ b/clang/lib/CodeGen/CGExpr.cpp
@@ -668,9 +668,9 @@ bool CodeGenFunction::isVptrCheckRequired(TypeCheckKind 
TCK, QualType Ty) {
 }
 
 bool CodeGenFunction::sanitizePerformTypeCheck() const {
-  return SanOpts.has(SanitizerKind::Null) |
- SanOpts.has(SanitizerKind::Alignment) |
- SanOpts.has(SanitizerKind::ObjectSize) |
+  return SanOpts.has(SanitizerKind::Null) ||
+ SanOpts.has(SanitizerKind::Alignment) ||
+ SanOpts.has(SanitizerKind::ObjectSize) ||
  SanOpts.has(SanitizerKind::Vptr);
 }
 

diff  --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index 8b53e3504e13..615ba1aef8e6 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -2988,7 +2988,7 @@ bool Sema::CheckHexagonBuiltinArgument(unsigned 
BuiltinID, CallExpr *TheCall) {
   unsigned M = 1 << A.Align;
   Min *= M;
   Max *= M;
-  Error |= SemaBuiltinConstantArgRange(TheCall, A.OpNum, Min, Max) |
+  Error |= SemaBuiltinConstantArgRange(TheCall, A.OpNum, Min, Max) ||
SemaBuiltinConstantArgMultiple(TheCall, A.OpNum, M);
 }
   }

diff  --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index 8d483f317a42..aedfc07c466d 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -8377,7 +8377,7 @@ QualType Sema::CheckConditionalOperands(ExprResult , 
ExprResult ,
   // OpenCL v2.0 s6.12.5 - Blocks cannot be used as expressions of the ternary
   // selection operator (?:).
   if (getLangOpts().OpenCL &&
-  (checkBlockType(*this, LHS.get()) | checkBlockType(*this, RHS.get( {
+  ((int)checkBlockType(*this, LHS.get()) | (int)checkBlockType(*this, 
RHS.get( {
 return QualType();
   }
 

diff  --git a/llvm/unittests/Support/TargetParserTest.cpp 
b/llvm/unittests/Support/TargetParserTest.cpp
index a465eb0c57c7..de63efdf7276 100644
--- a/llvm/unittests/Support/TargetParserTest.cpp
+++ b/llvm/unittests/Support/TargetParserTest.cpp
@@ -1181,9 +1181,9 @@ TEST(TargetParserTest, testAArch64CPUArchList) {
 bool testAArch64Arch(StringRef Arch, StringRef DefaultCPU, StringRef SubArch,
  unsigned ArchAttr) {
   AArch64::ArchKind AK = AArch64::parseArch(Arch);
-  return (AK != AArch64::ArchKind::INVALID) &
- AArch64::getDefaultCPU(Arch).equals(DefaultCPU) &
- AArch64::getSubArch(AK).equals(SubArch) &
+  return (AK != AArch64::ArchKind::INVALID) &&
+ AArch64::getDefaultCPU(Arch).equals(DefaultCPU) &&
+ AArch64::getSubArch(AK).equals(SubArch) &&
  (AArch64::getArchAttr(AK) == ArchAttr);
 }
 

diff  --git a/llvm/utils/TableGen/CodeGenDAGPatterns.cpp 
b/llvm/utils/TableGen/CodeGenDAGPatterns.cpp
index c1a3a34d928b..ebb4a31d86f4 100644
--- a/llvm/utils/TableGen/CodeGenDAGPatterns.cpp
+++ b/llvm/utils/TableGen/CodeGenDAGPatterns.cpp
@@ -1612,8 +1612,10 @@ bool 
SDTypeConstraint::ApplyTypeConstraint(TreePatternNode *N,
 unsigned OResNo = 0;
 TreePatternNode *OtherNode =
   getOperandNum(x.SDTCisSameAs_Info.OtherOperandNum, N, NodeInfo, OResNo);
-return NodeToApply->UpdateNodeType(ResNo, 
OtherNode->getExtType(OResNo),TP)|
-   OtherNode->UpdateNodeType(OResNo,NodeToApply->getExtType(ResNo),TP);
+return (int)NodeToApply->UpdateNodeType(ResNo,
+OtherNode->getExtType(OResNo), TP) 
|
+   (int)OtherNode->UpdateNodeType(OResNo,
+

[PATCH] D111000: [clang-format] allow clang-format to be passed a file of filenames so we can add a regression suite of "clean clang-formatted files" from LLVM

2021-10-03 Thread MyDeveloperDay via Phabricator via cfe-commits 
MyDeveloperDay added a comment.

In D111000#3038308 , 
@HazardyKnusperkeks wrote:

> Basically okay, I would have made 3 commits out of it:
>
> 1. Add the function to clang-format
> 2. The code clean up of the python script
> 3. The additional file generation

Yeah sorry about that, on their own I felt they were a little odd as to why I 
was making them, the formatting changes is because not I'm trying to run 
autopep8 and pylint


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111000

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

[clang] a4933f5 - Revert "[Clang] Extend -Wbool-operation to warn about bitwise and of bools with side effects"

2021-10-03 Thread Dávid Bolvanský via cfe-commits 

Author: Dávid Bolvanský
Date: 2021-10-03T12:47:12+02:00
New Revision: a4933f57f3f0a45e1db1075f7285f0761a80fc06

URL: 
https://github.com/llvm/llvm-project/commit/a4933f57f3f0a45e1db1075f7285f0761a80fc06
DIFF: 
https://github.com/llvm/llvm-project/commit/a4933f57f3f0a45e1db1075f7285f0761a80fc06.diff

LOG: Revert "[Clang] Extend -Wbool-operation to warn about bitwise and of bools 
with side effects"

This reverts commit f62d18ff140f67a8776a7a3c62a75645d8d540b5. Found some cases 
in LLVM itself.

Added: 


Modified: 
clang/include/clang/Basic/DiagnosticGroups.td
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/lib/Sema/SemaChecking.cpp
clang/test/Misc/warning-wall.c

Removed: 
clang/test/Sema/warn-bitwise-and-bool.c
clang/test/Sema/warn-bitwise-or-bool.c



diff  --git a/clang/include/clang/Basic/DiagnosticGroups.td 
b/clang/include/clang/Basic/DiagnosticGroups.td
index d9db3482dbda7..761b323d06166 100644
--- a/clang/include/clang/Basic/DiagnosticGroups.td
+++ b/clang/include/clang/Basic/DiagnosticGroups.td
@@ -64,8 +64,7 @@ def StringConversion : DiagGroup<"string-conversion">;
 def SignConversion : DiagGroup<"sign-conversion">;
 def PointerBoolConversion : DiagGroup<"pointer-bool-conversion">;
 def UndefinedBoolConversion : DiagGroup<"undefined-bool-conversion">;
-def BitwiseInsteadOfLogical : DiagGroup<"bitwise-instead-of-logical">;
-def BoolOperation : DiagGroup<"bool-operation", [BitwiseInsteadOfLogical]>;
+def BoolOperation : DiagGroup<"bool-operation">;
 def BoolConversion : DiagGroup<"bool-conversion", [PointerBoolConversion,
UndefinedBoolConversion]>;
 def IntConversion : DiagGroup<"int-conversion">;

diff  --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td 
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index c71a00b184328..1d4ea92c65205 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -66,7 +66,6 @@ def warn_infinite_recursive_function : Warning<
 def warn_comma_operator : Warning<"possible misuse of comma operator here">,
   InGroup>, DefaultIgnore;
 def note_cast_to_void : Note<"cast expression to void to silence warning">;
-def note_cast_operand_to_int : Note<"cast one or both operands to int to 
silence this warning">;
 
 // Constant expressions
 def err_expr_not_ice : Error<
@@ -7424,9 +7423,6 @@ def note_member_declared_here : Note<
   "member %0 declared here">;
 def note_member_first_declared_here : Note<
   "member %0 first declared here">;
-def warn_bitwise_instead_of_logical : Warning<
-  "use of bitwise '%0' with boolean operands">,
-  InGroup, DefaultIgnore;
 def warn_bitwise_negation_bool : Warning<
   "bitwise negation of a boolean expression%select{;| always evaluates to 
'true';}0 "
   "did you mean logical negation?">,

diff  --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index 10a3f30704172..8b53e3504e13a 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -13249,20 +13249,6 @@ static void AnalyzeImplicitConversions(
   << OrigE->getSourceRange() << T->isBooleanType()
   << FixItHint::CreateReplacement(UO->getBeginLoc(), "!");
 
-  if (const auto *BO = dyn_cast(SourceExpr))
-if ((BO->getOpcode() == BO_And || BO->getOpcode() == BO_Or) &&
-BO->getLHS()->isKnownToHaveBooleanValue() &&
-BO->getRHS()->isKnownToHaveBooleanValue() &&
-BO->getLHS()->HasSideEffects(S.Context) &&
-BO->getRHS()->HasSideEffects(S.Context)) {
-  S.Diag(BO->getBeginLoc(), diag::warn_bitwise_instead_of_logical)
-  << (BO->getOpcode() == BO_And ? "&" : "|") << OrigE->getSourceRange()
-  << FixItHint::CreateReplacement(
- BO->getOperatorLoc(),
- (BO->getOpcode() == BO_And ? "&&" : "||"));
-  S.Diag(BO->getBeginLoc(), diag::note_cast_operand_to_int);
-}
-
   // For conditional operators, we analyze the arguments as if they
   // were being fed directly into the output.
   if (auto *CO = dyn_cast(SourceExpr)) {

diff  --git a/clang/test/Misc/warning-wall.c b/clang/test/Misc/warning-wall.c
index a4a79bec934af..a3686fb96a4ce 100644
--- a/clang/test/Misc/warning-wall.c
+++ b/clang/test/Misc/warning-wall.c
@@ -4,7 +4,6 @@ RUN: FileCheck --input-file=%t %s
  CHECK:-Wall
 CHECK-NEXT:  -Wmost
 CHECK-NEXT:-Wbool-operation
-CHECK-NEXT:-Wbitwise-instead-of-logical
 CHECK-NEXT:-Wchar-subscripts
 CHECK-NEXT:-Wcomment
 CHECK-NEXT:-Wdelete-non-virtual-dtor

diff  --git a/clang/test/Sema/warn-bitwise-and-bool.c 
b/clang/test/Sema/warn-bitwise-and-bool.c
deleted file mode 100644
index 6bec1be1abdef..0
--- a/clang/test/Sema/warn-bitwise-and-bool.c
+++ /dev/null
@@ -1,63 +0,0 @@
-// RUN: %clang_cc1 -x c -fsyntax-only -verify -Wbool-operation %s
-// RUN: %clang_cc1 -x c -fsyntax-only

[PATCH] D108003: [Clang] Extend -Wbool-operation to warn about bitwise and of bools with side effects

2021-10-03 Thread Dávid Bolvanský via Phabricator via cfe-commits 
This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rGf62d18ff140f: [Clang] Extend -Wbool-operation to warn about 
bitwise and of bools with side… (authored by xbolva00).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108003

Files:
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaChecking.cpp
  clang/test/Misc/warning-wall.c
  clang/test/Sema/warn-bitwise-and-bool.c
  clang/test/Sema/warn-bitwise-or-bool.c

Index: clang/test/Sema/warn-bitwise-or-bool.c
===
--- /dev/null
+++ clang/test/Sema/warn-bitwise-or-bool.c
@@ -0,0 +1,63 @@
+// RUN: %clang_cc1 -x c -fsyntax-only -verify -Wbool-operation %s
+// RUN: %clang_cc1 -x c -fsyntax-only -verify -Wall %s
+// RUN: %clang_cc1 -x c -fsyntax-only -Wbitwise-instead-of-logical -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s
+// RUN: %clang_cc1 -x c -fsyntax-only -Wbool-operation -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s
+// RUN: %clang_cc1 -x c++ -fsyntax-only -verify -Wbool-operation %s
+// RUN: %clang_cc1 -x c++ -fsyntax-only -verify -Wall %s
+// RUN: %clang_cc1 -x c++ -fsyntax-only -Wbitwise-instead-of-logical -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s
+// RUN: %clang_cc1 -x c++ -fsyntax-only -Wbool-operation -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s
+
+#ifdef __cplusplus
+typedef bool boolean;
+#else
+typedef _Bool boolean;
+#endif
+
+boolean foo(void);
+boolean bar(void);
+boolean baz(void) __attribute__((const));
+void sink(boolean);
+
+#define FOO foo()
+
+void test(boolean a, boolean b, int *p, volatile int *q, int i) {
+  b = a | b;
+  b = foo() | a;
+  b = (p != 0) | (*p == 42);   // FIXME: also warn for a non-volatile pointer dereference
+  b = foo() | (*q == 42);  // expected-warning {{use of bitwise '|' with boolean operands}}
+   // expected-note@-1 {{cast one or both operands to int to silence this warning}}
+  b = foo() | (int)(*q == 42); // OK, no warning expected
+  b = a | foo();
+  b = (int)a | foo(); // OK, no warning expected
+  b = foo() | bar();  // expected-warning {{use of bitwise '|' with boolean operands}}
+  // expected-note@-1 {{cast one or both operands to int to silence this warning}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-2]]:13-[[@LINE-2]]:14}:"||"
+  b = foo() | !bar(); // expected-warning {{use of bitwise '|' with boolean operands}}
+  // expected-note@-1 {{cast one or both operands to int to silence this warning}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-2]]:13-[[@LINE-2]]:14}:"||"
+  b = foo() | (int)bar(); // OK, no warning expected
+  b = a | baz();
+  b = bar() | FOO;// expected-warning {{use of bitwise '|' with boolean operands}}
+  // expected-note@-1 {{cast one or both operands to int to silence this warning}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-2]]:13-[[@LINE-2]]:14}:"||"
+  b = foo() | (int)FOO;   // OK, no warning expected
+  b = b | foo();
+  b = bar() | (i > 4);
+  b = (i == 7) | foo();
+#ifdef __cplusplus
+  b = foo() bitor bar();  // expected-warning {{use of bitwise '|' with boolean operands}}
+  // expected-note@-1 {{cast one or both operands to int to silence this warning}}
+#endif
+
+  if (foo() | bar())  // expected-warning {{use of bitwise '|' with boolean operands}}
+  // expected-note@-1 {{cast one or both operands to int to silence this warning}}
+;
+
+  sink(a | b);
+  sink(a | foo());
+  sink(foo() | bar());// expected-warning {{use of bitwise '|' with boolean operands}}
+  // expected-note@-1 {{cast one or both operands to int to silence this warning}}
+
+  int n = i + 10;
+  b = (n | (n - 1));
+}
Index: clang/test/Sema/warn-bitwise-and-bool.c
===
--- /dev/null
+++ clang/test/Sema/warn-bitwise-and-bool.c
@@ -0,0 +1,63 @@
+// RUN: %clang_cc1 -x c -fsyntax-only -verify -Wbool-operation %s
+// RUN: %clang_cc1 -x c -fsyntax-only -verify -Wall %s
+// RUN: %clang_cc1 -x c -fsyntax-only -Wbitwise-instead-of-logical -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s
+// RUN: %clang_cc1 -x c -fsyntax-only -Wbool-operation -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s
+// RUN: %clang_cc1 -x c++ -fsyntax-only -verify -Wbool-operation %s
+// RUN: %clang_cc1 -x c++ -fsyntax-only -verify -Wall %s
+// RUN: %clang_cc1 -x c++ -fsyntax-only -Wbitwise-instead-of-logical -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s
+// RUN: %clang_cc1 -x c++ -fsyntax-only -Wbool-operation

[clang] f62d18f - [Clang] Extend -Wbool-operation to warn about bitwise and of bools with side effects

2021-10-03 Thread Dávid Bolvanský via cfe-commits 

Author: Dávid Bolvanský
Date: 2021-10-03T11:06:40+02:00
New Revision: f62d18ff140f67a8776a7a3c62a75645d8d540b5

URL: 
https://github.com/llvm/llvm-project/commit/f62d18ff140f67a8776a7a3c62a75645d8d540b5
DIFF: 
https://github.com/llvm/llvm-project/commit/f62d18ff140f67a8776a7a3c62a75645d8d540b5.diff

LOG: [Clang] Extend -Wbool-operation to warn about bitwise and of bools with 
side effects

Motivation: 
https://arstechnica.com/gadgets/2021/07/google-pushed-a-one-character-typo-to-production-bricking-chrome-os-devices/

Warn for pattern boolA & boolB or boolA | boolB where boolA and boolB has 
possible side effects.

Casting one operand to int is enough to silence this warning: for example 
(int)boolA & boolB or boolA| (int)boolB

Fixes https://bugs.llvm.org/show_bug.cgi?id=51216

Differential Revision: https://reviews.llvm.org/D108003

Added: 
clang/test/Sema/warn-bitwise-and-bool.c
clang/test/Sema/warn-bitwise-or-bool.c

Modified: 
clang/include/clang/Basic/DiagnosticGroups.td
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/lib/Sema/SemaChecking.cpp
clang/test/Misc/warning-wall.c

Removed: 




diff  --git a/clang/include/clang/Basic/DiagnosticGroups.td 
b/clang/include/clang/Basic/DiagnosticGroups.td
index 761b323d06166..d9db3482dbda7 100644
--- a/clang/include/clang/Basic/DiagnosticGroups.td
+++ b/clang/include/clang/Basic/DiagnosticGroups.td
@@ -64,7 +64,8 @@ def StringConversion : DiagGroup<"string-conversion">;
 def SignConversion : DiagGroup<"sign-conversion">;
 def PointerBoolConversion : DiagGroup<"pointer-bool-conversion">;
 def UndefinedBoolConversion : DiagGroup<"undefined-bool-conversion">;
-def BoolOperation : DiagGroup<"bool-operation">;
+def BitwiseInsteadOfLogical : DiagGroup<"bitwise-instead-of-logical">;
+def BoolOperation : DiagGroup<"bool-operation", [BitwiseInsteadOfLogical]>;
 def BoolConversion : DiagGroup<"bool-conversion", [PointerBoolConversion,
UndefinedBoolConversion]>;
 def IntConversion : DiagGroup<"int-conversion">;

diff  --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td 
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 1d4ea92c65205..c71a00b184328 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -66,6 +66,7 @@ def warn_infinite_recursive_function : Warning<
 def warn_comma_operator : Warning<"possible misuse of comma operator here">,
   InGroup>, DefaultIgnore;
 def note_cast_to_void : Note<"cast expression to void to silence warning">;
+def note_cast_operand_to_int : Note<"cast one or both operands to int to 
silence this warning">;
 
 // Constant expressions
 def err_expr_not_ice : Error<
@@ -7423,6 +7424,9 @@ def note_member_declared_here : Note<
   "member %0 declared here">;
 def note_member_first_declared_here : Note<
   "member %0 first declared here">;
+def warn_bitwise_instead_of_logical : Warning<
+  "use of bitwise '%0' with boolean operands">,
+  InGroup, DefaultIgnore;
 def warn_bitwise_negation_bool : Warning<
   "bitwise negation of a boolean expression%select{;| always evaluates to 
'true';}0 "
   "did you mean logical negation?">,

diff  --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index 8b53e3504e13a..10a3f30704172 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -13249,6 +13249,20 @@ static void AnalyzeImplicitConversions(
   << OrigE->getSourceRange() << T->isBooleanType()
   << FixItHint::CreateReplacement(UO->getBeginLoc(), "!");
 
+  if (const auto *BO = dyn_cast(SourceExpr))
+if ((BO->getOpcode() == BO_And || BO->getOpcode() == BO_Or) &&
+BO->getLHS()->isKnownToHaveBooleanValue() &&
+BO->getRHS()->isKnownToHaveBooleanValue() &&
+BO->getLHS()->HasSideEffects(S.Context) &&
+BO->getRHS()->HasSideEffects(S.Context)) {
+  S.Diag(BO->getBeginLoc(), diag::warn_bitwise_instead_of_logical)
+  << (BO->getOpcode() == BO_And ? "&" : "|") << OrigE->getSourceRange()
+  << FixItHint::CreateReplacement(
+ BO->getOperatorLoc(),
+ (BO->getOpcode() == BO_And ? "&&" : "||"));
+  S.Diag(BO->getBeginLoc(), diag::note_cast_operand_to_int);
+}
+
   // For conditional operators, we analyze the arguments as if they
   // were being fed directly into the output.
   if (auto *CO = dyn_cast(SourceExpr)) {

diff  --git a/clang/test/Misc/warning-wall.c b/clang/test/Misc/warning-wall.c
index a3686fb96a4ce..a4a79bec934af 100644
--- a/clang/test/Misc/warning-wall.c
+++ b/clang/test/Misc/warning-wall.c
@@ -4,6 +4,7 @@ RUN: FileCheck --input-file=%t %s
  CHECK:-Wall
 CHECK-NEXT:  -Wmost
 CHECK-NEXT:-Wbool-operation
+CHECK-NEXT:-Wbitwise-instead-of-logical
 CHECK-NEXT:-Wchar-subscripts
 CHECK-NEXT:-Wcomment
 CHECK-NEXT:

[PATCH] D110614: [clang-tidy] Fix false positives in cppcoreguidelines-virtual-class-destructor

2021-10-03 Thread Carlos Galvez via Phabricator via cfe-commits 
carlosgalvezp marked 3 inline comments as done.
carlosgalvezp added a comment.

Hi! Are there any more issues I should address? It would be nice to get it 
merged since it fixes quite a few FPs.

Tagging also @mgartmann as original author of this check in case he wants to 
comment.

Anyway this check will need to be extended in the future, since the C++ Core 
Guidelines has added a new bullet in their "Enforcement" section:
https://github.com/isocpp/CppCoreGuidelines/commit/e44a9fcbd40923e9d5d342e444cf8a811e4a3eae


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

https://reviews.llvm.org/D110614

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