[clang] [lldb] [llvm] Remove some `try_compile` CMake checks for compiler flags (PR #92953)

2024-05-23 Thread Felipe de Azevedo Piovezan via cfe-commits

felipepiovezan wrote:

These types of changes touching a lot of projects at once can benefit from 
multiple PRs, one per project, as it makes partial reverts a lot easier and 
doesn't cause as much churn downstream (plus we can get more targeted comments 
from individual project owners)

https://github.com/llvm/llvm-project/pull/92953
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang-tools-extra] [compiler-rt] [flang] [libc] [libclc] [libcxx] [libcxxabi] [libunwind] [lld] [lldb] [llvm] [mlir] [openmp] [polly] [pstl] Update IDE Folders (PR #89153)

2024-04-22 Thread Felipe de Azevedo Piovezan via cfe-commits


@@ -406,5 +426,13 @@ function(llvm_ExternalProject_Add name source_dir)
   WORKING_DIRECTORY ${BINARY_DIR}
   VERBATIM
   USES_TERMINAL)
+if (ARG_FOLDER)
+  set_target_properties(${target} PROPERTIES FOLDER "${ARG_FOLDER}")
+endif ()
   endforeach()
+
+  #set_target_folder(
+  #  TARGETS ${name} install-${name} ${name}-clobber ${name}-clear 
${name}-clean ${ARG_EXTRA_TARGETS}
+  #  FOLDER "${ARG_FOLDE}"
+  #)

felipepiovezan wrote:

commented code?

https://github.com/llvm/llvm-project/pull/89153
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang-tools-extra] [compiler-rt] [flang] [libclc] [libcxx] [lld] [lldb] [llvm] [NFC] Remove trailing whitespace across all non-test related files (PR #82838)

2024-02-26 Thread Felipe de Azevedo Piovezan via cfe-commits

felipepiovezan wrote:

> > This seems akin to running clang-format on the entire project, which last 
> > time we talked about still faced opposition
> 
> My impression (I admit I haven't reviewed the whole thread lately) is that 
> the opposition has mostly to do with how clang-format mangles some 
> constructs, not to the idea in general.
> 
> I'm juggling 4 projects at the moment, I have to finish some internal 
> release-related work before I can get back to clang-format-all-the-things.

Sorry, you might be right; my intent with that comment was not to a make broad 
summary of the thread, but rather to point out the thread's existence and that 
this PR should probably be folded into that discussion.

https://github.com/llvm/llvm-project/pull/82838
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang-tools-extra] [compiler-rt] [flang] [libclc] [libcxx] [lld] [lldb] [llvm] [NFC] Remove trailing whitespace across all non-test related files (PR #82838)

2024-02-23 Thread Felipe de Azevedo Piovezan via cfe-commits

felipepiovezan wrote:

Juuust to make sure, was this discussed before? This seems akin to running 
clang-format on the entire project, which last time we talked about still faced 
opposition: 
https://discourse.llvm.org/t/rfc-clang-format-all-the-things/76614/11

https://github.com/llvm/llvm-project/pull/82838
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] Add support for builtin_verbose_trap (PR #79230)

2024-01-25 Thread Felipe de Azevedo Piovezan via cfe-commits

https://github.com/felipepiovezan edited 
https://github.com/llvm/llvm-project/pull/79230
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] Add support for builtin_verbose_trap (PR #79230)

2024-01-25 Thread Felipe de Azevedo Piovezan via cfe-commits


@@ -3416,6 +3437,27 @@ llvm::DIMacroFile 
*CGDebugInfo::CreateTempMacroFile(llvm::DIMacroFile *Parent,
   return DBuilder.createTempMacroFile(Parent, Line, FName);
 }
 
+llvm::DILocation *CGDebugInfo::CreateTrapFailureMessageFor(
+llvm::DebugLoc TrapLocation, StringRef Prefix, StringRef FailureMsg) {
+  // Create debug info that describes a fake function whose name is the failure
+  // message.
+  std::string FuncName(Prefix);
+  if (!FailureMsg.empty()) {
+// A space in the function name identifies this as not being a real 
function
+// because it's not a valid symbol name.
+FuncName += ": ";
+FuncName += FailureMsg;
+  }
+
+  assert(FuncName.size() > 0);
+  assert(FuncName.find(' ') != std::string::npos &&

felipepiovezan wrote:

We can better express this intent by moving the assert _before_ the `if` and by 
rewriting it as the faster `assert(!FailureMessage.empty() || FuncName.find(' 
') != npos)`

https://github.com/llvm/llvm-project/pull/79230
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] Add support for builtin_verbose_trap (PR #79230)

2024-01-24 Thread Felipe de Azevedo Piovezan via cfe-commits

https://github.com/felipepiovezan edited 
https://github.com/llvm/llvm-project/pull/79230
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] Add support for builtin_verbose_trap (PR #79230)

2024-01-24 Thread Felipe de Azevedo Piovezan via cfe-commits


@@ -3416,6 +3437,27 @@ llvm::DIMacroFile 
*CGDebugInfo::CreateTempMacroFile(llvm::DIMacroFile *Parent,
   return DBuilder.createTempMacroFile(Parent, Line, FName);
 }
 
+llvm::DILocation *CGDebugInfo::CreateTrapFailureMessageFor(
+llvm::DebugLoc TrapLocation, StringRef Prefix, StringRef FailureMsg) {
+  // Create debug info that describes a fake function whose name is the failure
+  // message.
+  std::string FuncName(Prefix);
+  if (!FailureMsg.empty()) {
+// A space in the function name identifies this as not being a real 
function
+// because it's not a valid symbol name.
+FuncName += ": ";
+FuncName += FailureMsg;
+  }
+
+  assert(FuncName.size() > 0);
+  assert(FuncName.find(' ') != std::string::npos &&

felipepiovezan wrote:

the pre condition (as stated in the function documentation) is that `Prefix` 
should have a space.
But we add a space whenever `FailureMsg` is not empty, so this assert is not 
catching a pre condition violation in all cases

https://github.com/llvm/llvm-project/pull/79230
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[lldb] [clang] fixing issue #64441 (PR #74814)

2023-12-08 Thread Felipe de Azevedo Piovezan via cfe-commits

https://github.com/felipepiovezan requested changes to this pull request.

Hi @jeevanghimire, thank you for the PR! I believe the original issue was not 
about changing LLVM test inputs, but rather about changing clang-tidy to warn 
about this kind of patterns. 
Please see my rationale in a previous comment for why we should not be changing 
tests.

For future contributions, I'd also recommend using a more descriptive commit 
title and message. If you inspect the git log of the project, we generally 
follow the pattern:

[some topic]  




For example, this commit message could have been:

[clang-tidy] Implement "using namespace" check


https://github.com/llvm/llvm-project/pull/74814
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [lldb] fixing issue #64441 (PR #74814)

2023-12-08 Thread Felipe de Azevedo Piovezan via cfe-commits


@@ -34,7 +34,7 @@ namespace A {
 int myfunc (int a);
 int myfunc2(int a)
 {
- return a + 2;
+return a + 2; //just changing tab not much

felipepiovezan wrote:

In general, we don't add diffs that are unrelated to the problem being solved 
by the PR.
On a similar note, this comment seems to be trying to communicate with 
reviewers, not future readers of this test file. As such, this is the kind of 
comment that should be part of the commit message / PR description, and not 
part of the test itself

https://github.com/llvm/llvm-project/pull/74814
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [lldb] fixing issue #64441 (PR #74814)

2023-12-08 Thread Felipe de Azevedo Piovezan via cfe-commits


@@ -56,10 +56,10 @@ namespace Foo = A::B;   // namespace alias
 
 using Foo::myfunc;  // using declaration
 
-using namespace Foo;// using directive
+//removing namespace foo; for quality naming 

felipepiovezan wrote:

Hi @jeevanghimire, please note that this is a test, so we must ensure that LLDB 
does the right thing regardless of the input it receives, even if said input is 
not considered best practices. The test was likely created to either ensure 
LLDB works in the case, or because it didn't use to work and  someone created a 
test at the same time the fix was done.

 As such, I don't think we should change this.


https://github.com/llvm/llvm-project/pull/74814
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang] Avoid memcopy for small structure with padding under -ftrivial-auto-var-init (PR #71677)

2023-11-27 Thread Felipe de Azevedo Piovezan via cfe-commits

felipepiovezan wrote:

If this gets reverted for some reason, please note that I had to update a 
cross-project-tests test:
https://github.com/llvm/llvm-project/pull/73566

https://github.com/llvm/llvm-project/pull/71677
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang][DebugInfo] Fix iterator invalidation during EmitGlobalVariable (PR #72415)

2023-11-15 Thread Felipe de Azevedo Piovezan via cfe-commits

https://github.com/felipepiovezan edited 
https://github.com/llvm/llvm-project/pull/72415
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang][DebugInfo] Fix iterator invalidation during EmitGlobalVariable (PR #72415)

2023-11-15 Thread Felipe de Azevedo Piovezan via cfe-commits

https://github.com/felipepiovezan approved this pull request.

LGTM. If there are others waiting on the fix, and since this is a fairly 
trivial change, you may want to push it soonish

https://github.com/llvm/llvm-project/pull/72415
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [llvm] Add RunTimeLang to Class and Enumeration DIBuilder functions (PR #72011)

2023-11-14 Thread Felipe de Azevedo Piovezan via cfe-commits


@@ -424,6 +424,7 @@ namespace llvm {
 /// \param OffsetInBits Member offset.
 /// \param FlagsFlags to encode member attribute, e.g. private
 /// \param Elements class members.
+/// \param RunTimeLang  Optional parameter, Objective-C runtime version.

felipepiovezan wrote:

Ah, that's fair then

https://github.com/llvm/llvm-project/pull/72011
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [llvm] Add RunTimeLang to Class and Enumeration DIBuilder functions (PR #72011)

2023-11-14 Thread Felipe de Azevedo Piovezan via cfe-commits

felipepiovezan wrote:

> @felipepiovezan I don't this patch as it is has any changes that are 
> testable. I haven't added any code that will emit a RuntimeLang, only added 
> the option to do that so I can add those callers downstream. In retrospect I 
> should've marked it as NFC.

I see. I think this is yet another argument for flipping the argument order: if 
the new thing were the last argument, it would have been even more evident this 
is NFC :) 

https://github.com/llvm/llvm-project/pull/72011
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[llvm] [clang] Add RunTimeLang to Class and Enumeration DIBuilder functions (PR #72011)

2023-11-14 Thread Felipe de Azevedo Piovezan via cfe-commits

https://github.com/felipepiovezan edited 
https://github.com/llvm/llvm-project/pull/72011
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [llvm] Add RunTimeLang to Class and Enumeration DIBuilder functions (PR #72011)

2023-11-14 Thread Felipe de Azevedo Piovezan via cfe-commits

https://github.com/felipepiovezan edited 
https://github.com/llvm/llvm-project/pull/72011
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [llvm] Add RunTimeLang to Class and Enumeration DIBuilder functions (PR #72011)

2023-11-14 Thread Felipe de Azevedo Piovezan via cfe-commits


@@ -424,6 +424,7 @@ namespace llvm {
 /// \param OffsetInBits Member offset.
 /// \param FlagsFlags to encode member attribute, e.g. private
 /// \param Elements class members.
+/// \param RunTimeLang  Optional parameter, Objective-C runtime version.

felipepiovezan wrote:

let's use an `optional` type to denote an optional value

https://github.com/llvm/llvm-project/pull/72011
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [llvm] Add RunTimeLang to Class and Enumeration DIBuilder functions (PR #72011)

2023-11-14 Thread Felipe de Azevedo Piovezan via cfe-commits


@@ -424,6 +424,7 @@ namespace llvm {
 /// \param OffsetInBits Member offset.
 /// \param FlagsFlags to encode member attribute, e.g. private
 /// \param Elements class members.
+/// \param RunTimeLang  Optional parameter, Objective-C runtime version.

felipepiovezan wrote:

I think your entire patch would be a lot simpler if this were the last 
parameter of the function, as you wouldn't need to change as many callsites and 
have inline comments explaining the parameter. Did you have a motivation in 
mind for placing the argument here?

https://github.com/llvm/llvm-project/pull/72011
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[llvm] [clang] Add RunTimeLang to Class and Enumeration DIBuilder functions (PR #72011)

2023-11-14 Thread Felipe de Azevedo Piovezan via cfe-commits

https://github.com/felipepiovezan edited 
https://github.com/llvm/llvm-project/pull/72011
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[llvm] [clang] Add RunTimeLang to Class and Enumeration DIBuilder functions (PR #72011)

2023-11-14 Thread Felipe de Azevedo Piovezan via cfe-commits

https://github.com/felipepiovezan commented:

Let's add the `[DebugInfo][CGDebugInfo]` tags the commit message so that the 
git log  is more searchable.

I think we should have at least one test in this patch as well

https://github.com/llvm/llvm-project/pull/72011
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang][DebugInfo][NFC] Add createConstantValueExpression helper (PR #70674)

2023-10-30 Thread Felipe de Azevedo Piovezan via cfe-commits


@@ -5935,3 +5918,28 @@ llvm::DINode::DIFlags 
CGDebugInfo::getCallSiteRelatedAttrs() const {
 
   return llvm::DINode::FlagAllCallsDescribed;
 }
+
+llvm::DIExpression *
+CGDebugInfo::createConstantValueExpression(clang::ValueDecl const *VD,
+   const APValue ) {

felipepiovezan wrote:

`const` is a  inconsistent here, right? We have both east and west const

https://github.com/llvm/llvm-project/pull/70674
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang][DebugInfo][NFC] Add createConstantValueExpression helper (PR #70674)

2023-10-30 Thread Felipe de Azevedo Piovezan via cfe-commits


@@ -5935,3 +5918,28 @@ llvm::DINode::DIFlags 
CGDebugInfo::getCallSiteRelatedAttrs() const {
 
   return llvm::DINode::FlagAllCallsDescribed;
 }
+
+llvm::DIExpression *
+CGDebugInfo::createConstantValueExpression(clang::ValueDecl const *VD,
+   const APValue ) {
+  llvm::DIExpression *ValExpr = nullptr;

felipepiovezan wrote:

I know this is how the original code was written, but since we are making a 
separate function we are now in a position where we can delete this line, 
early-return on the `if` and replace all assignments to `ValExpr` with return 
statements.

https://github.com/llvm/llvm-project/pull/70674
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang][DebugInfo][NFC] Add createConstantValueExpression helper (PR #70674)

2023-10-30 Thread Felipe de Azevedo Piovezan via cfe-commits

https://github.com/felipepiovezan approved this pull request.

LGTM but left a couple of comments.
Thanks for breaking this into two separate PRs.

https://github.com/llvm/llvm-project/pull/70674
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang][DebugInfo][NFC] Add createConstantValueExpression helper (PR #70674)

2023-10-30 Thread Felipe de Azevedo Piovezan via cfe-commits

https://github.com/felipepiovezan edited 
https://github.com/llvm/llvm-project/pull/70674
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [IPSCCP] Variable not visible at Og. (PR #66745)

2023-10-04 Thread Felipe de Azevedo Piovezan via cfe-commits


@@ -106,14 +107,71 @@ static void findReturnsToZap(Function ,
   }
 }
 
-static bool runIPSCCP(
-Module , const DataLayout , FunctionAnalysisManager *FAM,
-std::function GetTLI,
-std::function GetTTI,
-std::function GetAC,
-std::function GetDT,
-std::function GetBFI,
-bool IsFuncSpecEnabled) {
+static void createDebugConstantExpression(Module , GlobalVariable *GV) {
+  SmallVector GVEs;
+  GV->getDebugInfo(GVEs);
+  if (GVEs.size() != 1)
+return;
+
+  DIBuilder DIB(M);
+
+  // Create integer constant expression.
+  auto createIntegerExpression = [](const Constant *CV) -> DIExpression * {
+const APInt  = cast(CV)->getValue();
+std::optional InitIntOpt;
+if (API.isNonNegative())
+  InitIntOpt = API.tryZExtValue();
+else if (auto Temp = API.trySExtValue())
+  // Transform a signed optional to unsigned optional.
+  InitIntOpt = static_cast(*Temp);

felipepiovezan wrote:

Looking at this again, isn't this the equivalent of always doing a sign 
extension?
We are basically saying: if this starts with a zero, add zeros; if this starts 
with a one, add ones.
Unless I am missing something, this code could be:

```
if (std::optional InitIntOpt = API.trySExtValue())
   return DIB.createConstantValueExpression(static_cast(*InitIntOpt))

return nullptr;
```

https://github.com/llvm/llvm-project/pull/66745
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang-tools-extra] [IPSCCP] Variable not visible at Og. (PR #66745)

2023-10-04 Thread Felipe de Azevedo Piovezan via cfe-commits


@@ -106,14 +107,71 @@ static void findReturnsToZap(Function ,
   }
 }
 
-static bool runIPSCCP(
-Module , const DataLayout , FunctionAnalysisManager *FAM,
-std::function GetTLI,
-std::function GetTTI,
-std::function GetAC,
-std::function GetDT,
-std::function GetBFI,
-bool IsFuncSpecEnabled) {
+static void createDebugConstantExpression(Module , GlobalVariable *GV) {
+  SmallVector GVEs;
+  GV->getDebugInfo(GVEs);
+  if (GVEs.size() != 1)
+return;
+
+  DIBuilder DIB(M);
+
+  // Create integer constant expression.
+  auto createIntegerExpression = [](const Constant *CV) -> DIExpression * {
+const APInt  = cast(CV)->getValue();
+std::optional InitIntOpt;
+if (API.isNonNegative())
+  InitIntOpt = API.tryZExtValue();
+else if (auto Temp = API.trySExtValue())
+  // Transform a signed optional to unsigned optional.
+  InitIntOpt = static_cast(*Temp);

felipepiovezan wrote:

Looking at this again, isn't this the equivalent of always doing a sign 
extension?
We are basically saying: if this starts with a zero, add zeros; if this starts 
with a one, add ones.
Unless I am missing something, this code could be:

```
if (std::optional InitIntOpt = API.trySExtValue())
   return DIB.createConstantValueExpression(static_cast(*InitIntOpt))

return nullptr;
```

https://github.com/llvm/llvm-project/pull/66745
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [IPSCCP] Variable not visible at Og. (PR #66745)

2023-10-04 Thread Felipe de Azevedo Piovezan via cfe-commits


@@ -106,14 +107,71 @@ static void findReturnsToZap(Function ,
   }
 }
 
-static bool runIPSCCP(
-Module , const DataLayout , FunctionAnalysisManager *FAM,
-std::function GetTLI,
-std::function GetTTI,
-std::function GetAC,
-std::function GetDT,
-std::function GetBFI,
-bool IsFuncSpecEnabled) {
+static void createDebugConstantExpression(Module , GlobalVariable *GV) {
+  SmallVector GVEs;
+  GV->getDebugInfo(GVEs);
+  if (GVEs.size() != 1)
+return;
+
+  DIBuilder DIB(M);
+
+  // Create integer constant expression.
+  auto createIntegerExpression = [](const Constant *CV) -> DIExpression * {
+const APInt  = cast(CV)->getValue();
+std::optional InitIntOpt;
+if (API.isNonNegative())
+  InitIntOpt = API.tryZExtValue();
+else if (auto Temp = API.trySExtValue())
+  // Transform a signed optional to unsigned optional.
+  InitIntOpt = static_cast(*Temp);
+return InitIntOpt ? DIB.createConstantValueExpression(InitIntOpt.value())
+  : nullptr;
+  };
+
+  const Constant *CV = GV->getInitializer();
+  Type *Ty = GV->getValueType();
+  if (Ty->isIntegerTy()) {
+DIExpression *InitExpr = createIntegerExpression(CV);
+if (InitExpr)
+  GVEs[0]->replaceOperandWith(1, InitExpr);
+return;
+  }
+
+  if (Ty->isFloatTy() || Ty->isDoubleTy()) {
+const APFloat  = cast(CV)->getValueAPF();
+GVEs[0]->replaceOperandWith(1, DIB.createConstantValueExpression(
+   APF.bitcastToAPInt().getZExtValue()));
+return;
+  }
+
+  if (!Ty->isPointerTy())
+return;
+
+  if (isa(CV)) {
+GVEs[0]->replaceOperandWith(1, DIB.createConstantValueExpression(0));
+return;
+  }
+  if (const ConstantExpr *CE = dyn_cast(CV);
+  CE->getNumOperands() == 1) {
+const Value *V = CE->getOperand(0);
+const Constant *CV = dyn_cast(V);
+if (CV && !isa(CV);

felipepiovezan wrote:

Yeah, I believe the intent here would be:

```
if (auto CI = dyn_cast_or_null(V)) {
  ...
}
```

Note that `!isa(CV)` is not necessary: a global value is always a 
pointer, therefore it cannot be a `ConstantInt`

https://github.com/llvm/llvm-project/pull/66745
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang-tools-extra] [IPSCCP] Variable not visible at Og. (PR #66745)

2023-10-04 Thread Felipe de Azevedo Piovezan via cfe-commits


@@ -106,14 +107,71 @@ static void findReturnsToZap(Function ,
   }
 }
 
-static bool runIPSCCP(
-Module , const DataLayout , FunctionAnalysisManager *FAM,
-std::function GetTLI,
-std::function GetTTI,
-std::function GetAC,
-std::function GetDT,
-std::function GetBFI,
-bool IsFuncSpecEnabled) {
+static void createDebugConstantExpression(Module , GlobalVariable *GV) {
+  SmallVector GVEs;
+  GV->getDebugInfo(GVEs);
+  if (GVEs.size() != 1)
+return;
+
+  DIBuilder DIB(M);
+
+  // Create integer constant expression.
+  auto createIntegerExpression = [](const Constant *CV) -> DIExpression * {
+const APInt  = cast(CV)->getValue();
+std::optional InitIntOpt;
+if (API.isNonNegative())
+  InitIntOpt = API.tryZExtValue();
+else if (auto Temp = API.trySExtValue())
+  // Transform a signed optional to unsigned optional.
+  InitIntOpt = static_cast(*Temp);
+return InitIntOpt ? DIB.createConstantValueExpression(InitIntOpt.value())
+  : nullptr;
+  };
+
+  const Constant *CV = GV->getInitializer();
+  Type *Ty = GV->getValueType();
+  if (Ty->isIntegerTy()) {
+DIExpression *InitExpr = createIntegerExpression(CV);
+if (InitExpr)
+  GVEs[0]->replaceOperandWith(1, InitExpr);
+return;
+  }
+
+  if (Ty->isFloatTy() || Ty->isDoubleTy()) {
+const APFloat  = cast(CV)->getValueAPF();
+GVEs[0]->replaceOperandWith(1, DIB.createConstantValueExpression(
+   APF.bitcastToAPInt().getZExtValue()));
+return;
+  }
+
+  if (!Ty->isPointerTy())
+return;
+
+  if (isa(CV)) {
+GVEs[0]->replaceOperandWith(1, DIB.createConstantValueExpression(0));
+return;
+  }
+  if (const ConstantExpr *CE = dyn_cast(CV);
+  CE->getNumOperands() == 1) {
+const Value *V = CE->getOperand(0);
+const Constant *CV = dyn_cast(V);
+if (CV && !isa(CV);

felipepiovezan wrote:

Yeah, I believe the intent here would be:

```
if (auto CI = dyn_cast_or_null(V)) {
  ...
}
```

Note that `!isa(CV)` is not necessary: a global value is always a 
pointer, therefore it cannot be a `ConstantInt`

https://github.com/llvm/llvm-project/pull/66745
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] Propagate the DWARF version from the main compiler invocation to PCHC… (PR #66032)

2023-09-12 Thread Felipe de Azevedo Piovezan via cfe-commits

https://github.com/felipepiovezan approved this pull request.

I've confirmed that this fixes the 
`lang/objc/modules-objc-property/TestModulesObjCProperty.py` failure we see in 
the matrix bot for DWARF 5!

https://github.com/llvm/llvm-project/pull/66032
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang-tools-extra] d820772 - Revert "[Serialization] Place command line defines in the correct file"

2023-03-24 Thread Felipe de Azevedo Piovezan via cfe-commits

Author: Felipe de Azevedo Piovezan
Date: 2023-03-24T14:49:47-04:00
New Revision: d820772ce17215c3e2c0f72d16b2d3f14a8745fb

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

LOG: Revert "[Serialization] Place command line defines in the correct file"

This reverts commit 72073fc95cd4793a853925ddc8cc3fb2118808a5.

Added: 


Modified: 
clang-tools-extra/clangd/index/SymbolCollector.cpp
clang/docs/ReleaseNotes.rst
clang/lib/Serialization/ASTReader.cpp
clang/lib/Serialization/ASTWriter.cpp
clang/test/PCH/ms-pch-macro.c

Removed: 
clang/test/PCH/macro-cmdline.c



diff  --git a/clang-tools-extra/clangd/index/SymbolCollector.cpp 
b/clang-tools-extra/clangd/index/SymbolCollector.cpp
index 519aceec15a18..3179810b1b185 100644
--- a/clang-tools-extra/clangd/index/SymbolCollector.cpp
+++ b/clang-tools-extra/clangd/index/SymbolCollector.cpp
@@ -687,10 +687,8 @@ bool SymbolCollector::handleMacroOccurrence(const 
IdentifierInfo *Name,
 
   const auto  = PP->getSourceManager();
   auto DefLoc = MI->getDefinitionLoc();
-  // Also avoid storing macros that aren't defined in any file, i.e. predefined
-  // macros like __DBL_MIN__ and those defined on the command line.
+  // Also avoid storing predefined macros like __DBL_MIN__.
   if (SM.isWrittenInBuiltinFile(DefLoc) ||
-  SM.isWrittenInCommandLineFile(DefLoc) ||
   Name->getName() == "__GCC_HAVE_DWARF2_CFI_ASM")
 return true;
 

diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 526ea11303708..1f18de92b920c 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -177,8 +177,8 @@ Improvements to Clang's diagnostics
 - Diagnostic notes and fix-its are now generated for ``ifunc``/``alias`` 
attributes
   which point to functions whose names are mangled.
 - Diagnostics relating to macros on the command line of a preprocessed assembly
-  file or precompiled header are now reported as coming from the file
-   instead of .
+  file are now reported as coming from the file  instead of
+  .
 - Clang constexpr evaluator now provides a more concise diagnostic when calling
   function pointer that is known to be null.
 - Clang now avoids duplicate warnings on unreachable ``[[fallthrough]];`` 
statements

diff  --git a/clang/lib/Serialization/ASTReader.cpp 
b/clang/lib/Serialization/ASTReader.cpp
index 6654df40010cb..0273fa1b839a5 100644
--- a/clang/lib/Serialization/ASTReader.cpp
+++ b/clang/lib/Serialization/ASTReader.cpp
@@ -654,10 +654,6 @@ static bool checkPreprocessorOptions(
   SmallVector ExistingMacroNames;
   collectMacroDefinitions(ExistingPPOpts, ExistingMacros, );
 
-  // Use a line marker to enter the  file, as the defines and
-  // undefines here will have come from the command line.
-  SuggestedPredefines += "# 1 \"\" 1\n";
-
   for (unsigned I = 0, N = ExistingMacroNames.size(); I != N; ++I) {
 // Dig out the macro definition in the existing preprocessor options.
 StringRef MacroName = ExistingMacroNames[I];
@@ -717,10 +713,6 @@ static bool checkPreprocessorOptions(
 }
 return true;
   }
-
-  // Leave the  file and return to .
-  SuggestedPredefines += "# 1 \"\" 2\n";
-
   if (Validation == OptionValidateStrictMatches) {
 // If strict matches are requested, don't tolerate any extra defines in
 // the AST file that are missing on the command line.
@@ -1587,13 +1579,8 @@ bool ASTReader::ReadSLocEntry(int ID) {
 auto Buffer = ReadBuffer(SLocEntryCursor, Name);
 if (!Buffer)
   return true;
-FileID FID = SourceMgr.createFileID(std::move(Buffer), FileCharacter, ID,
-BaseOffset + Offset, IncludeLoc);
-if (Record[3]) {
-  auto  =
-  const_cast(SourceMgr.getSLocEntry(FID).getFile());
-  FileInfo.setHasLineDirectives();
-}
+SourceMgr.createFileID(std::move(Buffer), FileCharacter, ID,
+   BaseOffset + Offset, IncludeLoc);
 break;
   }
 

diff  --git a/clang/lib/Serialization/ASTWriter.cpp 
b/clang/lib/Serialization/ASTWriter.cpp
index 31e44b52929f4..d89ee4e6199fc 100644
--- a/clang/lib/Serialization/ASTWriter.cpp
+++ b/clang/lib/Serialization/ASTWriter.cpp
@@ -,11 +,6 @@ void ASTWriter::AddString(StringRef Str, RecordDataImpl 
) {
 bool ASTWriter::PreparePathForOutput(SmallVectorImpl ) {
   assert(Context && "should have context when outputting path");
 
-  // Leave special file names as they are.
-  StringRef PathStr(Path.data(), Path.size());
-  if (PathStr == "" || PathStr == "")
-return false;
-
   bool Changed =
   cleanPathForOutput(Context->getSourceManager().getFileManager(), Path);
 

diff  --git a/clang/test/PCH/macro-cmdline.c b/clang/test/PCH/macro-cmdline.c
deleted file mode 100644
index 

[clang] 997dc7e - [debug-info][codegen] Prevent creation of self-referential SP node

2023-02-20 Thread Felipe de Azevedo Piovezan via cfe-commits

Author: Felipe de Azevedo Piovezan
Date: 2023-02-20T14:22:49-05:00
New Revision: 997dc7e00f49663b60a78e18df1dfecdf62a4172

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

LOG: [debug-info][codegen] Prevent creation of self-referential SP node

The function `CGDebugInfo::EmitFunctionDecl` is supposed to create a
declaration -- never a _definition_ -- of a subprogram. This is made
evident by the fact that the SPFlags never have the "Declaration" bit
set by that function.

However, when `EmitFunctionDecl` calls `DIBuilder::createFunction`, it
still tries to fill the "Declaration" argument by passing it the result
of `getFunctionDeclaration(D)`. This will query an internal cache of
previously created declarations and, for most code paths, we return
nullptr; all is good.

However, as reported in [0], there are pathological cases in which we
attempt to recreate a declaration, so the cache query succeeds,
resulting in a subprogram declaration whose declaration field points to
another declaration. Through a series of RAUWs, the declaration field
ends up pointing to the SP itself. Self-referential MDNodes can't be
`unique`, which causes the verifier to fail (declarations must be
`unique`).

We can argue that the caller should check the cache first, but this is
not a correctness issue (declarations are `unique` anyway). The bug is
that `CGDebugInfo::EmitFunctionDecl` should always pass `nullptr` to the
declaration argument of `DIBuilder::createFunction`, expressing the fact
that declarations don't point to other declarations. AFAICT this is not
something for which any reasonable meaning exists.

This seems a lot like a copy-paste mistake that has survived for ~10
years, since other places in this file have the exact same call almost
token-by-token.

I've tested this by compiling LLVMSupport with and without the patch, O2
and O0, and comparing the dwarfdump of the lib. The dumps are identical
modulo the attributes decl_file/producer/comp_dir.

[0]: https://github.com/llvm/llvm-project/issues/59241

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

Added: 
llvm/test/Verifier/disubprogram_declaration.ll

Modified: 
clang/lib/CodeGen/CGDebugInfo.cpp
llvm/docs/LangRef.rst
llvm/lib/IR/Verifier.cpp

Removed: 




diff  --git a/clang/lib/CodeGen/CGDebugInfo.cpp 
b/clang/lib/CodeGen/CGDebugInfo.cpp
index bb91b5116d71b..4dab595ada76b 100644
--- a/clang/lib/CodeGen/CGDebugInfo.cpp
+++ b/clang/lib/CodeGen/CGDebugInfo.cpp
@@ -4225,10 +4225,9 @@ void CGDebugInfo::EmitFunctionDecl(GlobalDecl GD, 
SourceLocation Loc,
 
   llvm::DINodeArray Annotations = CollectBTFDeclTagAnnotations(D);
   llvm::DISubroutineType *STy = getOrCreateFunctionType(D, FnType, Unit);
-  llvm::DISubprogram *SP =
-  DBuilder.createFunction(FDContext, Name, LinkageName, Unit, LineNo, STy,
-  ScopeLine, Flags, SPFlags, TParamsArray.get(),
-  getFunctionDeclaration(D), nullptr, Annotations);
+  llvm::DISubprogram *SP = DBuilder.createFunction(
+  FDContext, Name, LinkageName, Unit, LineNo, STy, ScopeLine, Flags,
+  SPFlags, TParamsArray.get(), nullptr, nullptr, Annotations);
 
   // Preserve btf_decl_tag attributes for parameters of extern functions
   // for BPF target. The parameters created in this loop are attached as

diff  --git a/llvm/docs/LangRef.rst b/llvm/docs/LangRef.rst
index 4052e58faa7fb..b4c9cff4821cc 100644
--- a/llvm/docs/LangRef.rst
+++ b/llvm/docs/LangRef.rst
@@ -5776,11 +5776,12 @@ retained, even if their IR counterparts are optimized 
out of the IR. The
 
 .. _DISubprogramDeclaration:
 
-When ``isDefinition: false``, subprograms describe a declaration in the type
-tree as opposed to a definition of a function.  If the scope is a composite
-type with an ODR ``identifier:`` and that does not set ``flags: DIFwdDecl``,
-then the subprogram declaration is uniqued based only on its ``linkageName:``
-and ``scope:``.
+When ``spFlags: DISPFlagDefinition`` is not present, subprograms describe a
+declaration in the type tree as opposed to a definition of a function. In this
+case, the ``declaration`` field must be empty. If the scope is a composite type
+with an ODR ``identifier:`` and that does not set ``flags: DIFwdDecl``, then
+the subprogram declaration is uniqued based only on its ``linkageName:`` and
+``scope:``.
 
 .. code-block:: text
 
@@ -5789,9 +5790,9 @@ and ``scope:``.
 }
 
 !0 = distinct !DISubprogram(name: "foo", linkageName: "_Zfoov", scope: !1,
-file: !2, line: 7, type: !3, isLocal: true,
-isDefinition: true, scopeLine: 8,
-containingType: !4,
+file: !2, line: 7, type: !3,
+

[clang] f84d30e - Reland "[codegen] Store address of indirect arguments on the stack"

2023-02-01 Thread Felipe de Azevedo Piovezan via cfe-commits

Author: Felipe de Azevedo Piovezan
Date: 2023-02-01T13:07:47-05:00
New Revision: f84d30e172b1f2365b1437b931d99156a76ed8f2

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

LOG: Reland "[codegen] Store address of indirect arguments on the stack"

The commit was reverted due to a regression in debug information of an
optimized code test in lldb. This has since been addressed by:

1. rGf753e5be8239: [LiveDebugValues] Allow EntryValue with OP_deref
expressions
2. rG055f2f04e658: [mem2reg][debuginfo] Handle op_deref when converting
dbg.declare

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

Added: 


Modified: 
clang/docs/ReleaseNotes.rst
clang/lib/CodeGen/CGDebugInfo.cpp
clang/lib/CodeGen/CGDebugInfo.h
clang/lib/CodeGen/CGDecl.cpp
clang/test/CodeGen/aarch64-ls64.c
clang/test/CodeGen/atomic-arm64.c
clang/test/CodeGenCXX/amdgcn-func-arg.cpp
clang/test/CodeGenCXX/debug-info.cpp
clang/test/CodeGenCXX/derived-to-base-conv.cpp
clang/test/CodeGenCoroutines/coro-params-exp-namespace.cpp
clang/test/CodeGenCoroutines/coro-params.cpp
clang/test/OpenMP/for_firstprivate_codegen.cpp
clang/test/OpenMP/parallel_firstprivate_codegen.cpp
clang/test/OpenMP/sections_firstprivate_codegen.cpp
clang/test/OpenMP/single_firstprivate_codegen.cpp
clang/test/OpenMP/target_teams_distribute_firstprivate_codegen.cpp

clang/test/OpenMP/target_teams_distribute_parallel_for_firstprivate_codegen.cpp

clang/test/OpenMP/target_teams_distribute_parallel_for_simd_firstprivate_codegen.cpp
clang/test/OpenMP/target_teams_distribute_simd_firstprivate_codegen.cpp
clang/test/OpenMP/teams_distribute_firstprivate_codegen.cpp
clang/test/OpenMP/teams_distribute_parallel_for_firstprivate_codegen.cpp

clang/test/OpenMP/teams_distribute_parallel_for_simd_firstprivate_codegen.cpp
clang/test/OpenMP/teams_distribute_simd_firstprivate_codegen.cpp
clang/test/OpenMP/teams_firstprivate_codegen.cpp

Removed: 




diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index e5d09ce61778e..9d9c5c6cd3f34 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -72,6 +72,9 @@ Improvements to Clang's diagnostics
 
 Non-comprehensive list of changes in this release
 -
+- Clang now saves the address of ABI-indirect function parameters on the stack,
+  improving the debug information available in programs compiled without
+  optimizations.
 
 New Compiler Flags
 --

diff  --git a/clang/lib/CodeGen/CGDebugInfo.cpp 
b/clang/lib/CodeGen/CGDebugInfo.cpp
index fafc0f8015987..bb91b5116d71b 100644
--- a/clang/lib/CodeGen/CGDebugInfo.cpp
+++ b/clang/lib/CodeGen/CGDebugInfo.cpp
@@ -4840,9 +4840,10 @@ void CGDebugInfo::EmitDeclareOfBlockDeclRefVariable(
 
 llvm::DILocalVariable *
 CGDebugInfo::EmitDeclareOfArgVariable(const VarDecl *VD, llvm::Value *AI,
-  unsigned ArgNo, CGBuilderTy ) {
+  unsigned ArgNo, CGBuilderTy ,
+  bool UsePointerValue) {
   assert(CGM.getCodeGenOpts().hasReducedDebugInfo());
-  return EmitDeclare(VD, AI, ArgNo, Builder);
+  return EmitDeclare(VD, AI, ArgNo, Builder, UsePointerValue);
 }
 
 namespace {

diff  --git a/clang/lib/CodeGen/CGDebugInfo.h b/clang/lib/CodeGen/CGDebugInfo.h
index d807fb7fad576..a70b72e2d1420 100644
--- a/clang/lib/CodeGen/CGDebugInfo.h
+++ b/clang/lib/CodeGen/CGDebugInfo.h
@@ -489,10 +489,9 @@ class CGDebugInfo {
 
   /// Emit call to \c llvm.dbg.declare for an argument variable
   /// declaration.
-  llvm::DILocalVariable *EmitDeclareOfArgVariable(const VarDecl *Decl,
-  llvm::Value *AI,
-  unsigned ArgNo,
-  CGBuilderTy );
+  llvm::DILocalVariable *
+  EmitDeclareOfArgVariable(const VarDecl *Decl, llvm::Value *AI, unsigned 
ArgNo,
+   CGBuilderTy , bool UsePointerValue = false);
 
   /// Emit call to \c llvm.dbg.declare for the block-literal argument
   /// to a block invocation function.

diff  --git a/clang/lib/CodeGen/CGDecl.cpp b/clang/lib/CodeGen/CGDecl.cpp
index ceaddc4e694ac..a70997f5b27b8 100644
--- a/clang/lib/CodeGen/CGDecl.cpp
+++ b/clang/lib/CodeGen/CGDecl.cpp
@@ -2476,6 +2476,8 @@ void CodeGenFunction::EmitParmDecl(const VarDecl , 
ParamValue Arg,
   Address AllocaPtr = Address::invalid();
   bool DoStore = false;
   bool IsScalar = hasScalarEvaluationKind(Ty);
+  bool UseIndirectDebugAddress = false;
+
   // If we already have a pointer to the argument, reuse the input pointer.
   if (Arg.isIndirect()) {
 // If we 

Re: [clang] f2d301f - Revert "[codegen] Store address of indirect arguments on the stack"

2023-01-17 Thread Felipe de Azevedo Piovezan via cfe-commits
Apologies, I’ll do it next time!

In the meantime, the reason was posted in D141381

> On Jan 16, 2023, at 13:59, Roman Lebedev  wrote:
> 
> Reminder to please always mention the reason for the revert/recommit
> in the commit message.
> 
> On Mon, Jan 16, 2023 at 7:05 PM Felipe de Azevedo Piovezan via
> cfe-commits  wrote:
>> 
>> 
>> Author: Felipe de Azevedo Piovezan
>> Date: 2023-01-16T13:05:22-03:00
>> New Revision: f2d301fe82869f881b86b51da7b4752972c66707
>> 
>> URL: 
>> https://github.com/llvm/llvm-project/commit/f2d301fe82869f881b86b51da7b4752972c66707
>> DIFF: 
>> https://github.com/llvm/llvm-project/commit/f2d301fe82869f881b86b51da7b4752972c66707.diff
>> 
>> LOG: Revert "[codegen] Store address of indirect arguments on the stack"
>> 
>> This reverts commit 7e4447a17db4a070f01c8f8a87505a4b2a1b0e3a.
>> 
>> Added:
>> 
>> 
>> Modified:
>>clang/docs/ReleaseNotes.rst
>>clang/lib/CodeGen/CGDebugInfo.cpp
>>clang/lib/CodeGen/CGDebugInfo.h
>>clang/lib/CodeGen/CGDecl.cpp
>>clang/test/CodeGen/aarch64-ls64.c
>>clang/test/CodeGen/atomic-arm64.c
>>clang/test/CodeGenCXX/amdgcn-func-arg.cpp
>>clang/test/CodeGenCXX/debug-info.cpp
>>clang/test/CodeGenCXX/derived-to-base-conv.cpp
>>clang/test/CodeGenCoroutines/coro-params-exp-namespace.cpp
>>clang/test/CodeGenCoroutines/coro-params.cpp
>>clang/test/OpenMP/for_firstprivate_codegen.cpp
>>clang/test/OpenMP/parallel_firstprivate_codegen.cpp
>>clang/test/OpenMP/sections_firstprivate_codegen.cpp
>>clang/test/OpenMP/single_firstprivate_codegen.cpp
>>clang/test/OpenMP/target_teams_distribute_firstprivate_codegen.cpp
>>
>> clang/test/OpenMP/target_teams_distribute_parallel_for_firstprivate_codegen.cpp
>>
>> clang/test/OpenMP/target_teams_distribute_parallel_for_simd_firstprivate_codegen.cpp
>>clang/test/OpenMP/target_teams_distribute_simd_firstprivate_codegen.cpp
>>clang/test/OpenMP/teams_distribute_firstprivate_codegen.cpp
>>clang/test/OpenMP/teams_distribute_parallel_for_firstprivate_codegen.cpp
>>
>> clang/test/OpenMP/teams_distribute_parallel_for_simd_firstprivate_codegen.cpp
>>clang/test/OpenMP/teams_distribute_simd_firstprivate_codegen.cpp
>>clang/test/OpenMP/teams_firstprivate_codegen.cpp
>> 
>> Removed:
>> 
>> 
>> 
>> 
>> diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
>> index 731d10ac64a3f..3bbab951e8a8c 100644
>> --- a/clang/docs/ReleaseNotes.rst
>> +++ b/clang/docs/ReleaseNotes.rst
>> @@ -500,9 +500,6 @@ Non-comprehensive list of changes in this release
>> - Clang can now generate a PCH when using ``-fdelayed-template-parsing`` for
>>   code with templates containing loop hint pragmas, OpenMP pragmas, and
>>   ``#pragma unused``.
>> -- Clang now saves the address of ABI-indirect function parameters on the 
>> stack,
>> -  improving the debug information available in programs compiled without
>> -  optimizations.
>> 
>> 
>> New Compiler Flags
>> 
>> diff  --git a/clang/lib/CodeGen/CGDebugInfo.cpp 
>> b/clang/lib/CodeGen/CGDebugInfo.cpp
>> index f2d558456fde4..f53a9d00ae5fd 100644
>> --- a/clang/lib/CodeGen/CGDebugInfo.cpp
>> +++ b/clang/lib/CodeGen/CGDebugInfo.cpp
>> @@ -4822,10 +4822,9 @@ void CGDebugInfo::EmitDeclareOfBlockDeclRefVariable(
>> 
>> llvm::DILocalVariable *
>> CGDebugInfo::EmitDeclareOfArgVariable(const VarDecl *VD, llvm::Value *AI,
>> -  unsigned ArgNo, CGBuilderTy ,
>> -  const bool UsePointerValue) {
>> +  unsigned ArgNo, CGBuilderTy ) 
>> {
>>   assert(CGM.getCodeGenOpts().hasReducedDebugInfo());
>> -  return EmitDeclare(VD, AI, ArgNo, Builder, UsePointerValue);
>> +  return EmitDeclare(VD, AI, ArgNo, Builder);
>> }
>> 
>> namespace {
>> 
>> diff  --git a/clang/lib/CodeGen/CGDebugInfo.h 
>> b/clang/lib/CodeGen/CGDebugInfo.h
>> index 10660a2550b56..95484a060cd88 100644
>> --- a/clang/lib/CodeGen/CGDebugInfo.h
>> +++ b/clang/lib/CodeGen/CGDebugInfo.h
>> @@ -487,9 +487,10 @@ class CGDebugInfo {
>> 
>>   /// Emit call to \c llvm.dbg.declare for an argument variable
>>   /// declaration.
>> -  llvm::DILocalVariable *
>> -  EmitDeclareOfArgVariable(const VarDecl *Decl, llvm::Value *AI, unsigned 
>> ArgNo,
>> -  

[clang] f2d301f - Revert "[codegen] Store address of indirect arguments on the stack"

2023-01-16 Thread Felipe de Azevedo Piovezan via cfe-commits

Author: Felipe de Azevedo Piovezan
Date: 2023-01-16T13:05:22-03:00
New Revision: f2d301fe82869f881b86b51da7b4752972c66707

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

LOG: Revert "[codegen] Store address of indirect arguments on the stack"

This reverts commit 7e4447a17db4a070f01c8f8a87505a4b2a1b0e3a.

Added: 


Modified: 
clang/docs/ReleaseNotes.rst
clang/lib/CodeGen/CGDebugInfo.cpp
clang/lib/CodeGen/CGDebugInfo.h
clang/lib/CodeGen/CGDecl.cpp
clang/test/CodeGen/aarch64-ls64.c
clang/test/CodeGen/atomic-arm64.c
clang/test/CodeGenCXX/amdgcn-func-arg.cpp
clang/test/CodeGenCXX/debug-info.cpp
clang/test/CodeGenCXX/derived-to-base-conv.cpp
clang/test/CodeGenCoroutines/coro-params-exp-namespace.cpp
clang/test/CodeGenCoroutines/coro-params.cpp
clang/test/OpenMP/for_firstprivate_codegen.cpp
clang/test/OpenMP/parallel_firstprivate_codegen.cpp
clang/test/OpenMP/sections_firstprivate_codegen.cpp
clang/test/OpenMP/single_firstprivate_codegen.cpp
clang/test/OpenMP/target_teams_distribute_firstprivate_codegen.cpp

clang/test/OpenMP/target_teams_distribute_parallel_for_firstprivate_codegen.cpp

clang/test/OpenMP/target_teams_distribute_parallel_for_simd_firstprivate_codegen.cpp
clang/test/OpenMP/target_teams_distribute_simd_firstprivate_codegen.cpp
clang/test/OpenMP/teams_distribute_firstprivate_codegen.cpp
clang/test/OpenMP/teams_distribute_parallel_for_firstprivate_codegen.cpp

clang/test/OpenMP/teams_distribute_parallel_for_simd_firstprivate_codegen.cpp
clang/test/OpenMP/teams_distribute_simd_firstprivate_codegen.cpp
clang/test/OpenMP/teams_firstprivate_codegen.cpp

Removed: 




diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 731d10ac64a3f..3bbab951e8a8c 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -500,9 +500,6 @@ Non-comprehensive list of changes in this release
 - Clang can now generate a PCH when using ``-fdelayed-template-parsing`` for
   code with templates containing loop hint pragmas, OpenMP pragmas, and
   ``#pragma unused``.
-- Clang now saves the address of ABI-indirect function parameters on the stack,
-  improving the debug information available in programs compiled without
-  optimizations.
 
 
 New Compiler Flags

diff  --git a/clang/lib/CodeGen/CGDebugInfo.cpp 
b/clang/lib/CodeGen/CGDebugInfo.cpp
index f2d558456fde4..f53a9d00ae5fd 100644
--- a/clang/lib/CodeGen/CGDebugInfo.cpp
+++ b/clang/lib/CodeGen/CGDebugInfo.cpp
@@ -4822,10 +4822,9 @@ void CGDebugInfo::EmitDeclareOfBlockDeclRefVariable(
 
 llvm::DILocalVariable *
 CGDebugInfo::EmitDeclareOfArgVariable(const VarDecl *VD, llvm::Value *AI,
-  unsigned ArgNo, CGBuilderTy ,
-  const bool UsePointerValue) {
+  unsigned ArgNo, CGBuilderTy ) {
   assert(CGM.getCodeGenOpts().hasReducedDebugInfo());
-  return EmitDeclare(VD, AI, ArgNo, Builder, UsePointerValue);
+  return EmitDeclare(VD, AI, ArgNo, Builder);
 }
 
 namespace {

diff  --git a/clang/lib/CodeGen/CGDebugInfo.h b/clang/lib/CodeGen/CGDebugInfo.h
index 10660a2550b56..95484a060cd88 100644
--- a/clang/lib/CodeGen/CGDebugInfo.h
+++ b/clang/lib/CodeGen/CGDebugInfo.h
@@ -487,9 +487,10 @@ class CGDebugInfo {
 
   /// Emit call to \c llvm.dbg.declare for an argument variable
   /// declaration.
-  llvm::DILocalVariable *
-  EmitDeclareOfArgVariable(const VarDecl *Decl, llvm::Value *AI, unsigned 
ArgNo,
-   CGBuilderTy , bool UsePointerValue = false);
+  llvm::DILocalVariable *EmitDeclareOfArgVariable(const VarDecl *Decl,
+  llvm::Value *AI,
+  unsigned ArgNo,
+  CGBuilderTy );
 
   /// Emit call to \c llvm.dbg.declare for the block-literal argument
   /// to a block invocation function.

diff  --git a/clang/lib/CodeGen/CGDecl.cpp b/clang/lib/CodeGen/CGDecl.cpp
index a70997f5b27b8..ceaddc4e694ac 100644
--- a/clang/lib/CodeGen/CGDecl.cpp
+++ b/clang/lib/CodeGen/CGDecl.cpp
@@ -2476,8 +2476,6 @@ void CodeGenFunction::EmitParmDecl(const VarDecl , 
ParamValue Arg,
   Address AllocaPtr = Address::invalid();
   bool DoStore = false;
   bool IsScalar = hasScalarEvaluationKind(Ty);
-  bool UseIndirectDebugAddress = false;
-
   // If we already have a pointer to the argument, reuse the input pointer.
   if (Arg.isIndirect()) {
 // If we have a prettier pointer type at this point, bitcast to that.
@@ -2489,19 +2487,6 @@ void CodeGenFunction::EmitParmDecl(const VarDecl , 
ParamValue Arg,
 auto AllocaAS = CGM.getASTAllocaAddressSpace();
 auto *V = 

[clang] 7e4447a - [codegen] Store address of indirect arguments on the stack

2023-01-16 Thread Felipe de Azevedo Piovezan via cfe-commits

Author: Felipe de Azevedo Piovezan
Date: 2023-01-16T11:14:55-03:00
New Revision: 7e4447a17db4a070f01c8f8a87505a4b2a1b0e3a

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

LOG: [codegen] Store address of indirect arguments on the stack

With codegen prior to this patch, truly indirect arguments -- i.e.
those that are not `byval` -- can have their debug information lost even
at O0. Because indirect arguments are passed by pointer, and this
pointer is likely placed in a register as per the function call ABI,
debug information is lost as soon as the register gets clobbered.

This patch solves the issue by storing the address of the parameter on
the stack, using a similar strategy employed when C++ references are
passed. In other words, this patch changes codegen from:

```
define @foo(ptr %arg) {
   call void @llvm.dbg.declare(%arg, [...], metadata !DIExpression())
```

To:

```
define @foo(ptr %arg) {
   %ptr_storage = alloca ptr
   store ptr %arg, ptr %ptr_storage
   call void @llvm.dbg.declare(%ptr_storage, [...], metadata 
!DIExpression(DW_OP_deref))
```

Some common cases where this may happen with C or C++ function calls:
  1. "Big enough" trivial structures passed by value under the ARM ABI.
  2. Structures that are non-trivial for the purposes of call (as per
  the Itanium ABI) when passed by value.

A few tests were matching the wrong alloca (matching against the new
alloca, instead of the old one), so they were updated to either match
both allocas or include a `,` right after the alloca type, to prevent
matching against a pointer type.

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

Added: 


Modified: 
clang/docs/ReleaseNotes.rst
clang/lib/CodeGen/CGDebugInfo.cpp
clang/lib/CodeGen/CGDebugInfo.h
clang/lib/CodeGen/CGDecl.cpp
clang/test/CodeGen/aarch64-ls64.c
clang/test/CodeGen/atomic-arm64.c
clang/test/CodeGenCXX/amdgcn-func-arg.cpp
clang/test/CodeGenCXX/debug-info.cpp
clang/test/CodeGenCXX/derived-to-base-conv.cpp
clang/test/CodeGenCoroutines/coro-params-exp-namespace.cpp
clang/test/CodeGenCoroutines/coro-params.cpp
clang/test/OpenMP/for_firstprivate_codegen.cpp
clang/test/OpenMP/parallel_firstprivate_codegen.cpp
clang/test/OpenMP/sections_firstprivate_codegen.cpp
clang/test/OpenMP/single_firstprivate_codegen.cpp
clang/test/OpenMP/target_teams_distribute_firstprivate_codegen.cpp

clang/test/OpenMP/target_teams_distribute_parallel_for_firstprivate_codegen.cpp

clang/test/OpenMP/target_teams_distribute_parallel_for_simd_firstprivate_codegen.cpp
clang/test/OpenMP/target_teams_distribute_simd_firstprivate_codegen.cpp
clang/test/OpenMP/teams_distribute_firstprivate_codegen.cpp
clang/test/OpenMP/teams_distribute_parallel_for_firstprivate_codegen.cpp

clang/test/OpenMP/teams_distribute_parallel_for_simd_firstprivate_codegen.cpp
clang/test/OpenMP/teams_distribute_simd_firstprivate_codegen.cpp
clang/test/OpenMP/teams_firstprivate_codegen.cpp

Removed: 




diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 3bbab951e8a8c..731d10ac64a3f 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -500,6 +500,9 @@ Non-comprehensive list of changes in this release
 - Clang can now generate a PCH when using ``-fdelayed-template-parsing`` for
   code with templates containing loop hint pragmas, OpenMP pragmas, and
   ``#pragma unused``.
+- Clang now saves the address of ABI-indirect function parameters on the stack,
+  improving the debug information available in programs compiled without
+  optimizations.
 
 
 New Compiler Flags

diff  --git a/clang/lib/CodeGen/CGDebugInfo.cpp 
b/clang/lib/CodeGen/CGDebugInfo.cpp
index f53a9d00ae5fd..f2d558456fde4 100644
--- a/clang/lib/CodeGen/CGDebugInfo.cpp
+++ b/clang/lib/CodeGen/CGDebugInfo.cpp
@@ -4822,9 +4822,10 @@ void CGDebugInfo::EmitDeclareOfBlockDeclRefVariable(
 
 llvm::DILocalVariable *
 CGDebugInfo::EmitDeclareOfArgVariable(const VarDecl *VD, llvm::Value *AI,
-  unsigned ArgNo, CGBuilderTy ) {
+  unsigned ArgNo, CGBuilderTy ,
+  const bool UsePointerValue) {
   assert(CGM.getCodeGenOpts().hasReducedDebugInfo());
-  return EmitDeclare(VD, AI, ArgNo, Builder);
+  return EmitDeclare(VD, AI, ArgNo, Builder, UsePointerValue);
 }
 
 namespace {

diff  --git a/clang/lib/CodeGen/CGDebugInfo.h b/clang/lib/CodeGen/CGDebugInfo.h
index 95484a060cd88..10660a2550b56 100644
--- a/clang/lib/CodeGen/CGDebugInfo.h
+++ b/clang/lib/CodeGen/CGDebugInfo.h
@@ -487,10 +487,9 @@ class CGDebugInfo {
 
   /// Emit call to \c llvm.dbg.declare for an argument variable
   /// declaration.
-  llvm::DILocalVariable