https://github.com/lamb-j edited https://github.com/llvm/llvm-project/pull/69371
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/lamb-j updated
https://github.com/llvm/llvm-project/pull/69371
>From 86535608c7e79d31260e2a358b7a00f7b747e8bd Mon Sep 17 00:00:00 2001
From: Jacob Lambert
Date: Tue, 17 Oct 2023 12:01:15 -0700
Subject: [PATCH 1/3] [NFC] Refactor BackendConsumer class definition into new
https://github.com/yxsamliu approved this pull request.
LGTM. Thanks
https://github.com/llvm/llvm-project/pull/69371
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
yxsamliu wrote:
sometimes the formatter checks the diff of the rebase when you rebase your
code. However if you rebase your commits against ToT of trunk the formatter
issue may disappear.
https://github.com/llvm/llvm-project/pull/69371
___
https://github.com/lamb-j updated
https://github.com/llvm/llvm-project/pull/69371
>From 25302c315360c166f34ab9acde2561dc7865b3bb Mon Sep 17 00:00:00 2001
From: Jacob Lambert
Date: Tue, 17 Oct 2023 12:01:15 -0700
Subject: [PATCH 1/3] [NFC] Refactor BackendConsumer class definition into new
jhuber6 wrote:
> That said, I definitely don't want this to be a barrier to getting this patch
> in, so if you still feel like we should go with the clang-format
> recommendation, I'll change it and also update the EmitAssembly and
> EmitBackendOutput signatures which were flagged by
lamb-j wrote:
> this doesn't really apply since you changed the function signature so it
> needs to be reformatted.
I don't follow the logic there. The function signature can have a style as
well. And I think this is actually a good example to demonstrate a reason not
to reformat if we look
jhuber6 wrote:
> > Just do what the formatter says, not every file is 100% clang-formatted so
> > there's bits of old code that haven't been properly cleaned yet. This was
> > the same line that I thought looked wrong so it should probably be fixed.
> > Using `git clang-format HEAD~1` only
lamb-j wrote:
>
> Just do what the formatter says, not every file is 100% clang-formatted so
> there's bits of old code that haven't been properly cleaned yet. This was the
> same line that I thought looked wrong so it should probably be fixed. Using
> `git clang-format HEAD~1` only formats
jhuber6 wrote:
> I am getting this from the formatter:
>
> ```
> - void RunOptimizationPipeline(BackendAction Action,
> - std::unique_ptr ,
> - std::unique_ptr ,
> - BackendConsumer *BC);
> + void
lamb-j wrote:
I am getting this from the formatter:
- void RunOptimizationPipeline(BackendAction Action,
- std::unique_ptr ,
- std::unique_ptr ,
- BackendConsumer *BC);
+ void RunOptimizationPipeline(
+
https://github.com/lamb-j updated
https://github.com/llvm/llvm-project/pull/69371
>From 25302c315360c166f34ab9acde2561dc7865b3bb Mon Sep 17 00:00:00 2001
From: Jacob Lambert
Date: Tue, 17 Oct 2023 12:01:15 -0700
Subject: [PATCH 1/3] [NFC] Refactor BackendConsumer class definition into new
jhuber6 wrote:
The code formatter says that it's not happy. Can you try `git clang-format
HEAD~1` in your branch?
https://github.com/llvm/llvm-project/pull/69371
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://github.com/jhuber6 approved this pull request.
I'm not entirely happy with the existence of this hack, but it's an ugly
solution to an ugly self-inflicted problem, so I can live with it for now.
https://github.com/llvm/llvm-project/pull/69371
@@ -0,0 +1,29 @@
+//===-- LinkInModulesPass.cpp - Module Linking pass --- C++
-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier:
https://github.com/lamb-j updated
https://github.com/llvm/llvm-project/pull/69371
>From 25302c315360c166f34ab9acde2561dc7865b3bb Mon Sep 17 00:00:00 2001
From: Jacob Lambert
Date: Tue, 17 Oct 2023 12:01:15 -0700
Subject: [PATCH 1/3] [NFC] Refactor BackendConsumer class definition into new
https://github.com/lamb-j updated
https://github.com/llvm/llvm-project/pull/69371
>From 25302c315360c166f34ab9acde2561dc7865b3bb Mon Sep 17 00:00:00 2001
From: Jacob Lambert
Date: Tue, 17 Oct 2023 12:01:15 -0700
Subject: [PATCH 1/3] [NFC] Refactor BackendConsumer class definition into new
@@ -0,0 +1,29 @@
+//===-- LinkInModulesPass.cpp - Module Linking pass --- C++
-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier:
https://github.com/lamb-j updated
https://github.com/llvm/llvm-project/pull/69371
>From 25302c315360c166f34ab9acde2561dc7865b3bb Mon Sep 17 00:00:00 2001
From: Jacob Lambert
Date: Tue, 17 Oct 2023 12:01:15 -0700
Subject: [PATCH 1/3] [NFC] Refactor BackendConsumer class definition into new
https://github.com/lamb-j edited https://github.com/llvm/llvm-project/pull/69371
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
@@ -48,428 +49,369 @@
#include "llvm/Support/ToolOutputFile.h"
#include "llvm/Support/YAMLTraits.h"
#include "llvm/Transforms/IPO/Internalize.h"
+#include "llvm/Transforms/Utils/Cloning.h"
-#include
#include
using namespace clang;
using namespace llvm;
#define
@@ -155,10 +162,10 @@ class EmitAssemblyHelper {
return F;
}
- void
- RunOptimizationPipeline(BackendAction Action,
+ void RunOptimizationPipeline(BackendAction Action,
std::unique_ptr ,
- std::unique_ptr );
+
@@ -155,10 +162,10 @@ class EmitAssemblyHelper {
return F;
}
- void
- RunOptimizationPipeline(BackendAction Action,
+ void RunOptimizationPipeline(BackendAction Action,
std::unique_ptr ,
- std::unique_ptr );
+
@@ -48,428 +49,369 @@
#include "llvm/Support/ToolOutputFile.h"
#include "llvm/Support/YAMLTraits.h"
#include "llvm/Transforms/IPO/Internalize.h"
+#include "llvm/Transforms/Utils/Cloning.h"
-#include
#include
using namespace clang;
using namespace llvm;
#define
@@ -45,7 +46,8 @@ namespace clang {
const TargetOptions , const LangOptions ,
StringRef TDesc, llvm::Module *M, BackendAction
Action,
llvm::IntrusiveRefCntPtr VFS,
-
@@ -1035,6 +1043,13 @@ void EmitAssemblyHelper::RunOptimizationPipeline(
}
}
+ // Re-link against any bitcodes supplied via the -mlink-builtin-bitcode
option
+ // Some optimizations may generate new function calls that would not have
+ // been linked
@@ -1035,6 +1043,13 @@ void EmitAssemblyHelper::RunOptimizationPipeline(
}
}
+ // Re-link against any bitcodes supplied via the -mlink-builtin-bitcode
option
+ // Some optimizations may generate new function calls that would not have
+ // been linked
@@ -1035,6 +1043,13 @@ void EmitAssemblyHelper::RunOptimizationPipeline(
}
}
+ // Re-link against any bitcodes supplied via the -mlink-builtin-bitcode
option
+ // Some optimizations may generate new function calls that would not have
+ // been linked
https://github.com/jhuber6 commented:
Some comments. I remember there was a reason we couldn't use the existing
linking support and needed the new pass, what was that again?
https://github.com/llvm/llvm-project/pull/69371
___
cfe-commits mailing list
@@ -1035,6 +1043,13 @@ void EmitAssemblyHelper::RunOptimizationPipeline(
}
}
+ // Re-link against any bitcodes supplied via the -mlink-builtin-bitcode
option
+ // Some optimizations may generate new function calls that would not have
+ // been linked
https://github.com/jhuber6 edited
https://github.com/llvm/llvm-project/pull/69371
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/lamb-j updated
https://github.com/llvm/llvm-project/pull/69371
>From 25302c315360c166f34ab9acde2561dc7865b3bb Mon Sep 17 00:00:00 2001
From: Jacob Lambert
Date: Tue, 17 Oct 2023 12:01:15 -0700
Subject: [PATCH 1/3] [NFC] Refactor BackendConsumer class definition into new
jhuber6 wrote:
> > You could potentially link in all the symbols and internalize them
>
> That probably won't work. After they are internalized, they have internal
> linkage and cannot be used to resolve newly added call of the same function.
>
> The purpose of internalization is to allow you
https://github.com/lamb-j updated
https://github.com/llvm/llvm-project/pull/69371
>From 25302c315360c166f34ab9acde2561dc7865b3bb Mon Sep 17 00:00:00 2001
From: Jacob Lambert
Date: Tue, 17 Oct 2023 12:01:15 -0700
Subject: [PATCH 1/3] [NFC] Refactor BackendConsumer class definition into new
yxsamliu wrote:
>
> You could potentially link in all the symbols and internalize them
That probably won't work. After they are internalized, they have internal
linkage and cannot be used to resolve newly added call of the same function.
The purpose of internalization is to allow you to
jhuber6 wrote:
> > This approach assumes that whatever the function call was transformed into
> > also exists in the same library, which isn't necessarily true.
>
> True, good point. But I don't think it's necessarily due to this approach,
> but more of how AMDGPULibCalls is implemented. It
lamb-j wrote:
> This approach assumes that whatever the function call was transformed into
> also exists in the same library, which isn't necessarily true.
True, good point. But I don't think it's necessarily due to this approach, but
more of how AMDGPULibCalls is implemented. It seems like
jhuber6 wrote:
> sincos() is just one example. There are several other cases that can trigger
> this issue. fold_rootn() generates new function calls for square and cubic
> roots, fold_pow() does a similar thing for specific powers (ex 2), etc.
>
> We did try disabling -amdgpu-prelink, and it
github-actions[bot] wrote:
:warning: C/C++ code formatter, clang-format found issues in your code.
:warning:
You can test this locally with the following command:
``bash
git-clang-format --diff fbf0a77e80f18a6d0fd8a28833b0bc87a99b1b2f
b7af255d2cb8f9a00d1977f92d4e3723ef0720bf --
lamb-j wrote:
sincos() is just one example. There are several other cases that can trigger
this issue. fold_rootn() generates new function calls for square and cubic
roots, fold_pow() does a similar thing for specific powers (ex 2)
We did try disabling -llvm-prelink, and it did lead to a
jhuber6 wrote:
> This handles the edge case where optimization introduces new device library
> functions, such as a fused sincos() from separate sin() and cos() calls.
This seems like a pretty big change to solve just this problem. Could we not
just put `nobuiltin` on said function to stop it
llvmbot wrote:
@llvm/pr-subscribers-clang
Author: Jacob Lambert (lamb-j)
Changes
This set of patches updates device library linking and optimization to do the
following:
Link
Optimize
Link (New)
This handles the edge case where optimization introduces new device library
functions,
https://github.com/lamb-j created
https://github.com/llvm/llvm-project/pull/69371
This set of patches updates device library linking and optimization to do the
following:
Link
Optimize
Link (New)
This handles the edge case where optimization introduces new device library
functions, such as
43 matches
Mail list logo
