[PATCH] D104556: [InstrProfiling] Make CountersPtr in __profd_ relative

2021-09-20 Thread Rich Kadel via Phabricator via cfe-commits 
richkadel added a comment.

I don't know if this is helpful or not, but we ran into a problem, recently, on 
Fuchsia when upgrading Clang (I believe to LLVM 14), and realized that the LLVM 
tools from Clang's version of LLVM were in compatible with the Rust profraw 
files. Since you're probably using a Rust compiler that's compatible with LLVM 
13 (if not earlier), you may need to make sure you're using Rust's bundled LLVM 
tools (`llvm-profdata` and `llvm-cov`) to process those files.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104556

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

[PATCH] D104556: [InstrProfiling] Make CountersPtr in __profd_ relative

2021-09-16 Thread Philippe Antoine via Phabricator via cfe-commits 
catenacyber added a comment.

Should I propose a simple patch to bump INSTR_PROF_RAW_VERSION ?
Or will it be part of something bigger, taken care of by someone else ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104556

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

[PATCH] D104556: [InstrProfiling] Make CountersPtr in __profd_ relative

2021-09-15 Thread Fangrui Song via Phabricator via cfe-commits 
MaskRay added a comment.

In D104556#3001497 , @catenacyber 
wrote:

> Should we still bump `INSTR_PROF_RAW_VERSION ` so that we are able to 
> distinguish profraw files produced by clang13 and the ones produced by 
> clang14 ?
>
> Right now, both produce `LLVM raw profile data, version 7`

Yes. 14.0.0 is far away, so there is plenty of time bumping it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104556

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

[PATCH] D104556: [InstrProfiling] Make CountersPtr in __profd_ relative

2021-09-15 Thread Philippe Antoine via Phabricator via cfe-commits 
catenacyber added a comment.

Should we still bump `INSTR_PROF_RAW_VERSION ` so that we are able to 
distinguish profraw files produced by clang13 and the ones produced by clang14 ?

Right now, both produce `LLVM raw profile data, version 7`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104556

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

[PATCH] D104556: [InstrProfiling] Make CountersPtr in __profd_ relative

2021-09-13 Thread Philippe Antoine via Phabricator via cfe-commits 
catenacyber added a comment.

> Even if `INSTR_PROF_RAW_VERSION` is bumped, I think it is very likely your 
> Rust downstream will observe an `if (GET_VERSION(Version) != 
> RawInstrProf::Version)` error in llvm-profdata,
> because from your description it seems you are likely mixing raw profile 
> files produced by compiler-rt/lib/profile built at different commits.

For the record, bumping `INSTR_PROF_RAW_VERSION` on top of main branch, (or 
after reverting this commit), in both 
compiler-rt/include/profile/InstrProfData.inc and 
llvm/include/llvm/ProfileData/InstrProfData.inc, I indeed get the error message 
`unsupported instrumentation profile format version`

The output for `file default.profraw` is `default.profraw: LLVM raw profile 
data, version 7`

> Bumping INSTR_PROF_RAW_VERSION will give a better diagnostic but won't 
> address the root cause that only one version is supported, which I totally 
> agree since otherwise this would add too much burden on the upstream LLVM 
> developer side.

Well, now I have my diagnostic :-) (it is not like 
https://github.com/rust-lang/rust/issues/82875 for instance)

> this is "Support for older format versions in RawInstrProfReader" 
> https://lists.llvm.org/pipermail/llvm-dev/2021-August/152287.html

Thanks for the pointer

> You'll need to rebuild everything on the Rust side if you want to use git 
> LLVM.

Indeed, I will try to rebuild rust with clang-14


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104556

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

[PATCH] D104556: [InstrProfiling] Make CountersPtr in __profd_ relative

2021-09-13 Thread Fangrui Song via Phabricator via cfe-commits 
MaskRay added a comment.

In D104556#2998159 , @catenacyber 
wrote:

>> .profraw are files with the raw profile format. The compiler-rt/lib/profile 
>> runtime and llvm-profdata only support one version at any commit.
>
> Ok.
>
>> Mixing .profraw files produced by different compiler-rt/lib/profile runtimes 
>> is unsupported.
>
> I do not think I am mixing profraw files as I have only one profraw file.
> If my llvm-profdata is not supporting the profraw file produced by rust + 
> linker, should it not error with
> ` Unsupported instrumentation profile format version` instead of `malformed 
> instrumentation profile data`
> As it does with rust nightly and clang 12 ?
>
>> I don't know what rustc and https://github.com/ctz/rustls do.
>
> Rustls is just an example rust project, I think we get the same result with 
> every rust project.

Wild guess:
this is "Support for older format versions in RawInstrProfReader" 
https://lists.llvm.org/pipermail/llvm-dev/2021-August/152287.html

>> I don't think this change is responsible as the change has been well 
>> released/tested by many C++ groups.
>
> I guess I did not mean to use the word responsible.
> I am just saying that if I `git revert 
> a1532ed27582038e2d9588108ba0fe8237f01844` and solve the conflicts on top of 
> main, I get my scenario to work again
>
>> If Rust adapts compiler-rt and does something different, I think the 
>> investigation responsibility is on Rust's side.
>>
>> Perhaps you can get some help from rustc folks who do the compiler-rt 
>> adaptation in rustc.
>
> I am asking indeed

The error only happens when you have mix-and-match .profraw files. You'll need 
to rebuild everything on the Rust side if you want to use git LLVM.
I guess that rustc itself or some prebuilt executable is using an old format.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104556

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

[PATCH] D104556: [InstrProfiling] Make CountersPtr in __profd_ relative

2021-09-13 Thread Philippe Antoine via Phabricator via cfe-commits 
catenacyber added a comment.

> .profraw are files with the raw profile format. The compiler-rt/lib/profile 
> runtime and llvm-profdata only support one version at any commit.

Ok.

> Mixing .profraw files produced by different compiler-rt/lib/profile runtimes 
> is unsupported.

I do not think I am mixing profraw files as I have only one profraw file.
If my llvm-profdata is not supporting the profraw file produced by rust + 
linker, should it not error with
` Unsupported instrumentation profile format version` instead of `malformed 
instrumentation profile data`
As it does with rust nightly and clang 12 ?

> I don't know what rustc and https://github.com/ctz/rustls do.

Rustls is just an example rust project, I think we get the same result with 
every rust project.

> I don't think this change is responsible as the change has been well 
> released/tested by many C++ groups.

I guess I did not mean to use the word responsible.
I am just saying that if I `git revert 
a1532ed27582038e2d9588108ba0fe8237f01844` and solve the conflicts on top of 
main, I get my scenario to work again

> If Rust adapts compiler-rt and does something different, I think the 
> investigation responsibility is on Rust's side.
>
> Perhaps you can get some help from rustc folks who do the compiler-rt 
> adaptation in rustc.

I am asking indeed


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104556

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

[PATCH] D104556: [InstrProfiling] Make CountersPtr in __profd_ relative

2021-09-13 Thread Fangrui Song via Phabricator via cfe-commits 
MaskRay added a comment.

In D104556#2998137 , @catenacyber 
wrote:

>> Bump the raw profile format version to 6. (Last bump happened in 2019-10.)
>
> This does not seem to be in the patch, ie no change of 
> `INSTR_PROF_RAW_VERSION` in compiler-rt/include/profile/InstrProfData.inc
> Is that missing ?

`INSTR_PROF_RAW_VERSION` is not bumped. It was bumped in the first revision of 
this patch if you looked at the history.

Somehow the binary ID change got pushed before this and it has been rolled 
back/forward several times in main.
The version is not updated to not cause unnecessary conflict with that patch.

Even if `INSTR_PROF_RAW_VERSION` is bumped, I think it is very likely your Rust 
downstream will observe an `if (GET_VERSION(Version) != RawInstrProf::Version)` 
error in llvm-profdata,
because from your description it seems you are likely mixing raw profile files 
produced by compiler-rt/lib/profile built at different commits.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104556

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

[PATCH] D104556: [InstrProfiling] Make CountersPtr in __profd_ relative

2021-09-13 Thread Philippe Antoine via Phabricator via cfe-commits 
catenacyber added a comment.

> Bump the raw profile format version to 6. (Last bump happened in 2019-10.)

This does not seem to be in the patch, ie no change of `INSTR_PROF_RAW_VERSION` 
in compiler-rt/include/profile/InstrProfData.inc
Is that missing ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104556

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

[PATCH] D104556: [InstrProfiling] Make CountersPtr in __profd_ relative

2021-09-13 Thread Fangrui Song via Phabricator via cfe-commits 
MaskRay added a comment.

In D104556#2998091 , @catenacyber 
wrote:

> Thanks for the quick reply
>
>> I don't think this change is responsible. This has been tested by many C++ 
>> downstream groups.
>
> What did I do wrong ?
> Adding this only commit makes the scenario fail.
>
>> Rust should use upgrade its lib/ProfileData and compiler-rt/lib/profile, and 
>> not mix raw profile files at different commits.
>
> I am not sure I understand.
> What are raw profiles ? 
> In my scenario, only `llvm-profdata` acts on default.profraw
> And before that, the linker, not the rust compiler, mixes together the 
> different coverage sections...
> So, how would Rust mix raw profiles ?

.profraw are files with the raw profile format. The compiler-rt/lib/profile 
runtime and llvm-profdata only support one version at any commit.
Mixing .profraw files produced by different compiler-rt/lib/profile runtimes is 
unsupported.

I don't know what rustc and https://github.com/ctz/rustls do.

I don't think this change is responsible as the change has been well 
released/tested by many C++ groups.
If Rust adapts compiler-rt and does something different, I think the 
investigation responsibility is on Rust's side.

Perhaps you can get some help from rustc folks who do the compiler-rt 
adaptation in rustc.

>> The binary ID change has caused a bit of churn to ProfileData but it is 
>> unrelated to this patch.
>
> Well, there may be other bugs, but this is not a problem in my scenario...




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104556

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

[PATCH] D104556: [InstrProfiling] Make CountersPtr in __profd_ relative

2021-09-13 Thread Philippe Antoine via Phabricator via cfe-commits 
catenacyber added a comment.

Thanks for the quick reply

> I don't think this change is responsible. This has been tested by many C++ 
> downstream groups.

What did I do wrong ?
Adding this only commit makes the scenario fail.

> Rust should use upgrade its lib/ProfileData and compiler-rt/lib/profile, and 
> not mix raw profile files at different commits.

I am not sure I understand.
What are raw profiles ? 
In my scenario, only `llvm-profdata` acts on default.profraw
And before that, the linker, not the rust compiler, mixes together the 
different coverage sections...
So, how would Rust mix raw profiles ?

> The binary ID change has caused a bit of churn to ProfileData but it is 
> unrelated to this patch.

Well, there may be other bugs, but this is not a problem in my scenario...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104556

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

[PATCH] D104556: [InstrProfiling] Make CountersPtr in __profd_ relative

2021-09-13 Thread Fangrui Song via Phabricator via cfe-commits 
MaskRay added a comment.

In D104556#2997674 , @catenacyber 
wrote:

> This commit has broken oss-fuzz workflow for rust projects.
> I do not know if the fix is 
> https://reviews.llvm.org/rG66e2772e4285588ccc3bcdb5f392c8326debbd7d
>
> Scenario is
>
>   export RUSTFLAGS="--cfg fuzzing -Cdebuginfo=1 -Cforce-frame-pointers 
> -Zinstrument-coverage -C link-dead-code -C link-arg=-lc++"
>   git clone https://github.com/ctz/rustls && cd rustls
>   cargo fuzz build -O
>   ./fuzz/target/x86_64-unknown-linux-gnu/release/client 
> ./fuzz/target/x86_64-unknown-linux-gnu/release/client
>   llvm-profdata merge -sparse default.profraw -o profdata
>
> Current output is
>
>   warning: default.profraw: malformed instrumentation profile data
>   error: no profile can be merged
>
> Scenario does not output error for in 13.0.0-rc2
>
> Error does not happen on commit 69cdadddecaf97f572c311aa07044f79a5b8329a 
>  just 
> previous this integration, if we add `git cherry-pick 
> a6c14fba70e170a279f7e77f068368f09d8c5eaf` to fix the other pending bug which 
> was fixed in 13.0.0-rc2
> Error happens on commit a1532ed27582038e2d9588108ba0fe8237f01844 
>  even if 
> we `git cherry-pick a6c14fba70e170a279f7e77f068368f09d8c5eaf` and fix the 
> conflict about only llvm/test/tools/llvm-profdata/Inputs/c-general.profraw

I don't think this change is responsible. This has been used by many C++ 
downstream groups.

Rust should use upgrade its lib/ProfileData and compiler-rt/lib/profile.
The binary ID change has caused a bit of churn to ProfileData but it is 
unrelated to this patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104556

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

[PATCH] D104556: [InstrProfiling] Make CountersPtr in __profd_ relative

2021-09-13 Thread Philippe Antoine via Phabricator via cfe-commits 
catenacyber added a comment.

This commit has broken oss-fuzz workflow for rust projects.
I do not know if the fix is 
https://reviews.llvm.org/rG66e2772e4285588ccc3bcdb5f392c8326debbd7d

Scenario is

  export RUSTFLAGS="--cfg fuzzing -Cdebuginfo=1 -Cforce-frame-pointers 
-Zinstrument-coverage -C link-dead-code -C link-arg=-lc++"
  git clone https://github.com/ctz/rustls && cd rustls
  cargo fuzz build -O
  ./fuzz/target/x86_64-unknown-linux-gnu/release/client 
./fuzz/target/x86_64-unknown-linux-gnu/release/client
  llvm-profdata merge -sparse default.profraw -o profdata

Current output is

  warning: default.profraw: malformed instrumentation profile data
  error: no profile can be merged

Scenario does not output error for in 13.0.0-rc2

Error does not happen on commit 69cdadddecaf97f572c311aa07044f79a5b8329a 
 just 
previous this integration, if we add `git cherry-pick 
a6c14fba70e170a279f7e77f068368f09d8c5eaf` to fix the other pending bug which 
was fixed in 13.0.0-rc2
Error happens on commit a1532ed27582038e2d9588108ba0fe8237f01844 
 even if 
we `git cherry-pick a6c14fba70e170a279f7e77f068368f09d8c5eaf` and fix the 
conflict about only llvm/test/tools/llvm-profdata/Inputs/c-general.profraw


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104556

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

[PATCH] D104556: [InstrProfiling] Make CountersPtr in __profd_ relative

2021-07-31 Thread Fangrui Song via Phabricator via cfe-commits 
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGa1532ed27582: [InstrProfiling] Make CountersPtr in __profd_ 
relative (authored by MaskRay).

Changed prior to commit:
  https://reviews.llvm.org/D104556?vs=356310=363171#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104556

Files:
  clang/test/Profile/c-linkage-available_externally.c
  compiler-rt/include/profile/InstrProfData.inc
  compiler-rt/lib/profile/InstrProfilingMerge.c
  compiler-rt/lib/profile/InstrProfilingWriter.c
  llvm/include/llvm/ProfileData/InstrProfData.inc
  llvm/lib/ProfileData/InstrProfReader.cpp
  llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp
  llvm/test/Instrumentation/InstrProfiling/icall.ll
  llvm/test/Instrumentation/InstrProfiling/profiling.ll
  llvm/test/Transforms/PGOProfile/comdat_internal.ll
  llvm/test/Transforms/PGOProfile/indirect_call_profile.ll
  llvm/test/Transforms/PGOProfile/memcpy.ll
  llvm/test/tools/llvm-profdata/Inputs/c-general.profraw
  llvm/test/tools/llvm-profdata/raw-32-bits-be.test
  llvm/test/tools/llvm-profdata/raw-32-bits-le.test
  llvm/test/tools/llvm-profdata/raw-64-bits-be.test
  llvm/test/tools/llvm-profdata/raw-64-bits-le.test

Index: llvm/test/tools/llvm-profdata/raw-64-bits-le.test
===
--- llvm/test/tools/llvm-profdata/raw-64-bits-le.test
+++ llvm/test/tools/llvm-profdata/raw-64-bits-le.test
@@ -19,7 +19,7 @@
 
 RUN: printf '\067\265\035\031\112\165\023\344' >> %t
 RUN: printf '\02\0\0\0\0\0\0\0' >> %t
-RUN: printf '\10\0\4\0\1\0\0\0' >> %t
+RUN: printf '\xd8\xff\3\0\1\0\0\0' >> %t
 RUN: printf '\0\0\0\0\0\0\0\0' >> %t
 RUN: printf '\0\0\0\0\0\0\0\0' >> %t
 RUN: printf '\02\0\0\0\0\0\0\0' >> %t
Index: llvm/test/tools/llvm-profdata/raw-64-bits-be.test
===
--- llvm/test/tools/llvm-profdata/raw-64-bits-be.test
+++ llvm/test/tools/llvm-profdata/raw-64-bits-be.test
@@ -19,7 +19,7 @@
 
 RUN: printf '\344\023\165\112\031\035\265\067' >> %t
 RUN: printf '\0\0\0\0\0\0\0\02' >> %t
-RUN: printf '\0\0\0\1\0\4\0\10' >> %t
+RUN: printf '\0\0\0\1\0\3\xff\xd8' >> %t
 RUN: printf '\0\0\0\0\0\0\0\0' >> %t
 RUN: printf '\0\0\0\0\0\0\0\0' >> %t
 RUN: printf '\0\0\0\02\0\0\0\0' >> %t
Index: llvm/test/tools/llvm-profdata/raw-32-bits-le.test
===
--- llvm/test/tools/llvm-profdata/raw-32-bits-le.test
+++ llvm/test/tools/llvm-profdata/raw-32-bits-le.test
@@ -20,7 +20,7 @@
 
 RUN: printf '\067\265\035\031\112\165\023\344' >> %t
 RUN: printf '\02\0\0\0\0\0\0\0' >> %t
-RUN: printf '\10\0\0\1' >> %t
+RUN: printf '\xe0\xff\xff\0' >> %t
 RUN: printf '\0\0\0\0' >> %t
 RUN: printf '\0\0\0\0' >> %t
 RUN: printf '\2\0\0\0' >> %t
Index: llvm/test/tools/llvm-profdata/raw-32-bits-be.test
===
--- llvm/test/tools/llvm-profdata/raw-32-bits-be.test
+++ llvm/test/tools/llvm-profdata/raw-32-bits-be.test
@@ -20,7 +20,7 @@
 
 RUN: printf '\344\023\165\112\031\035\265\067' >> %t
 RUN: printf '\0\0\0\0\0\0\0\2' >> %t
-RUN: printf '\1\0\0\10' >> %t
+RUN: printf '\0\xff\xff\xe0' >> %t
 RUN: printf '\0\0\0\0' >> %t
 RUN: printf '\0\0\0\0' >> %t
 RUN: printf '\0\0\0\2' >> %t
Index: llvm/test/Transforms/PGOProfile/memcpy.ll
===
--- llvm/test/Transforms/PGOProfile/memcpy.ll
+++ llvm/test/Transforms/PGOProfile/memcpy.ll
@@ -23,7 +23,7 @@
 
 for.body3:
   %conv = sext i32 %add to i64
-; CHECK: call void @__llvm_profile_instrument_memop(i64 %conv, i8* bitcast ({ i64, i64, i64*, i8*, i8*, i32, [2 x i16] }* @__profd_foo to i8*), i32 0)
+; CHECK: call void @__llvm_profile_instrument_memop(i64 %conv, i8* bitcast ({ i64, i64, i64, i8*, i8*, i32, [2 x i16] }* @__profd_foo to i8*), i32 0)
   call void @llvm.memcpy.p0i8.p0i8.i64(i8* %dst, i8* %src, i64 %conv, i1 false)
   %inc = add nsw i32 %j.0, 1
   br label %for.cond1
Index: llvm/test/Transforms/PGOProfile/indirect_call_profile.ll
===
--- llvm/test/Transforms/PGOProfile/indirect_call_profile.ll
+++ llvm/test/Transforms/PGOProfile/indirect_call_profile.ll
@@ -39,7 +39,7 @@
   %tmp = load void ()*, void ()** @bar, align 8
 ; GEN: [[ICALL_TARGET:%[0-9]+]] = ptrtoint void ()* %tmp to i64
 ; GEN-NEXT: call void @llvm.instrprof.value.profile(i8* getelementptr inbounds ([3 x i8], [3 x i8]* @__profn_foo, i32 0, i32 0), i64 [[#FOO_HASH]], i64 [[ICALL_TARGET]], i32 0, i32 0)
-; LOWER: call void @__llvm_profile_instrument_target(i64 %1, i8* bitcast ({ i64, i64, i64*, i8*, i8*, i32, [2 x i16] }* @__profd_foo to i8*), i32 0)
+; LOWER: call void @__llvm_profile_instrument_target(i64 %1, i8* bitcast ({ i64, i64, i64, i8*, i8*, i32, [2 x i16] }*

[PATCH] D104556: [InstrProfiling] Make CountersPtr in __profd_ relative

2021-07-29 Thread Fangrui Song via Phabricator via cfe-commits 
MaskRay added a comment.

release/13.x has been created. This if merged will go into 14.x. We have plenty 
of time making more format changes if needed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104556

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

[PATCH] D104556: [InstrProfiling] Make CountersPtr in __profd_ relative

2021-07-21 Thread Fangrui Song via Phabricator via cfe-commits 
MaskRay added a comment.

@davidxl @vsk 

I am happy to postpone this after 13.0.0 is branched - so that we can adjust 
the format while only bumping the version once.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104556

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

[PATCH] D104556: [InstrProfiling] Make CountersPtr in __profd_ relative

2021-07-05 Thread Fangrui Song via Phabricator via cfe-commits 
MaskRay added a comment.

@davidxl @vsk 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104556

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

[PATCH] D104556: [InstrProfiling] Make CountersPtr in __profd_ relative

2021-07-03 Thread Fangrui Song via Phabricator via cfe-commits 
MaskRay updated this revision to Diff 356310.
MaskRay marked an inline comment as done.
MaskRay added a comment.

Move ConstantExpr::getSub from .inc to InstrProfiling.cpp


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104556

Files:
  clang/test/Profile/c-linkage-available_externally.c
  compiler-rt/include/profile/InstrProfData.inc
  compiler-rt/lib/profile/InstrProfilingMerge.c
  compiler-rt/lib/profile/InstrProfilingWriter.c
  llvm/include/llvm/ProfileData/InstrProfData.inc
  llvm/lib/ProfileData/InstrProfReader.cpp
  llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp
  llvm/test/Instrumentation/InstrProfiling/icall.ll
  llvm/test/Instrumentation/InstrProfiling/profiling.ll
  llvm/test/Transforms/PGOProfile/comdat_internal.ll
  llvm/test/Transforms/PGOProfile/indirect_call_profile.ll
  llvm/test/Transforms/PGOProfile/memcpy.ll
  llvm/test/tools/llvm-profdata/Inputs/c-general.profraw
  llvm/test/tools/llvm-profdata/c-general.test
  llvm/test/tools/llvm-profdata/malformed-ptr-to-counter-array.test
  llvm/test/tools/llvm-profdata/raw-32-bits-be.test
  llvm/test/tools/llvm-profdata/raw-32-bits-le.test
  llvm/test/tools/llvm-profdata/raw-64-bits-be.test
  llvm/test/tools/llvm-profdata/raw-64-bits-le.test
  llvm/test/tools/llvm-profdata/raw-two-profiles.test

Index: llvm/test/tools/llvm-profdata/raw-two-profiles.test
===
--- llvm/test/tools/llvm-profdata/raw-two-profiles.test
+++ llvm/test/tools/llvm-profdata/raw-two-profiles.test
@@ -1,5 +1,5 @@
 RUN: printf '\201rforpl\377' > %t-foo.profraw
-RUN: printf '\5\0\0\0\0\0\0\0' >> %t-foo.profraw
+RUN: printf '\6\0\0\0\0\0\0\0' >> %t-foo.profraw
 RUN: printf '\1\0\0\0\0\0\0\0' >> %t-foo.profraw
 RUN: printf '\0\0\0\0\0\0\0\0' >> %t-foo.profraw
 RUN: printf '\1\0\0\0\0\0\0\0' >> %t-foo.profraw
@@ -20,7 +20,7 @@
 RUN: printf '\3\0foo\0\0\0' >> %t-foo.profraw
 
 RUN: printf '\201rforpl\377' > %t-bar.profraw
-RUN: printf '\5\0\0\0\0\0\0\0' >> %t-bar.profraw
+RUN: printf '\6\0\0\0\0\0\0\0' >> %t-bar.profraw
 RUN: printf '\1\0\0\0\0\0\0\0' >> %t-bar.profraw
 RUN: printf '\0\0\0\0\0\0\0\0' >> %t-bar.profraw
 RUN: printf '\2\0\0\0\0\0\0\0' >> %t-bar.profraw
Index: llvm/test/tools/llvm-profdata/raw-64-bits-le.test
===
--- llvm/test/tools/llvm-profdata/raw-64-bits-le.test
+++ llvm/test/tools/llvm-profdata/raw-64-bits-le.test
@@ -1,5 +1,5 @@
 RUN: printf '\201rforpl\377' > %t
-RUN: printf '\5\0\0\0\0\0\0\0' >> %t
+RUN: printf '\6\0\0\0\0\0\0\0' >> %t
 RUN: printf '\2\0\0\0\0\0\0\0' >> %t
 RUN: printf '\0\0\0\0\0\0\0\0' >> %t
 RUN: printf '\3\0\0\0\0\0\0\0' >> %t
@@ -18,7 +18,7 @@
 
 RUN: printf '\067\265\035\031\112\165\023\344' >> %t
 RUN: printf '\02\0\0\0\0\0\0\0' >> %t
-RUN: printf '\10\0\4\0\1\0\0\0' >> %t
+RUN: printf '\xd8\xff\3\0\1\0\0\0' >> %t
 RUN: printf '\0\0\0\0\0\0\0\0' >> %t
 RUN: printf '\0\0\0\0\0\0\0\0' >> %t
 RUN: printf '\02\0\0\0\0\0\0\0' >> %t
Index: llvm/test/tools/llvm-profdata/raw-64-bits-be.test
===
--- llvm/test/tools/llvm-profdata/raw-64-bits-be.test
+++ llvm/test/tools/llvm-profdata/raw-64-bits-be.test
@@ -1,5 +1,5 @@
 RUN: printf '\377lprofr\201' > %t
-RUN: printf '\0\0\0\0\0\0\0\5' >> %t
+RUN: printf '\0\0\0\0\0\0\0\6' >> %t
 RUN: printf '\0\0\0\0\0\0\0\2' >> %t
 RUN: printf '\0\0\0\0\0\0\0\0' >> %t
 RUN: printf '\0\0\0\0\0\0\0\3' >> %t
@@ -18,7 +18,7 @@
 
 RUN: printf '\344\023\165\112\031\035\265\067' >> %t
 RUN: printf '\0\0\0\0\0\0\0\02' >> %t
-RUN: printf '\0\0\0\1\0\4\0\10' >> %t
+RUN: printf '\0\0\0\1\0\3\xff\xd8' >> %t
 RUN: printf '\0\0\0\0\0\0\0\0' >> %t
 RUN: printf '\0\0\0\0\0\0\0\0' >> %t
 RUN: printf '\0\0\0\02\0\0\0\0' >> %t
Index: llvm/test/tools/llvm-profdata/raw-32-bits-le.test
===
--- llvm/test/tools/llvm-profdata/raw-32-bits-le.test
+++ llvm/test/tools/llvm-profdata/raw-32-bits-le.test
@@ -1,5 +1,5 @@
 RUN: printf '\201Rforpl\377' > %t
-RUN: printf '\5\0\0\0\0\0\0\0' >> %t
+RUN: printf '\6\0\0\0\0\0\0\0' >> %t
 RUN: printf '\2\0\0\0\0\0\0\0' >> %t
 RUN: printf '\0\0\0\0\0\0\0\0' >> %t
 RUN: printf '\3\0\0\0\0\0\0\0' >> %t
@@ -19,7 +19,7 @@
 
 RUN: printf '\067\265\035\031\112\165\023\344' >> %t
 RUN: printf '\02\0\0\0\0\0\0\0' >> %t
-RUN: printf '\10\0\0\1' >> %t
+RUN: printf '\xe0\xff\xff\0' >> %t
 RUN: printf '\0\0\0\0' >> %t
 RUN: printf '\0\0\0\0' >> %t
 RUN: printf '\2\0\0\0' >> %t
Index: llvm/test/tools/llvm-profdata/raw-32-bits-be.test
===
--- llvm/test/tools/llvm-profdata/raw-32-bits-be.test
+++ llvm/test/tools/llvm-profdata/raw-32-bits-be.test
@@ -1,5 +1,5 @@
 RUN: printf '\377lprofR\201' > %t
-RUN: printf '\0\0\0\0\0\0\0\5' >> %t
+RUN: printf '\0\0\0\0\0\0\0\6' >> %t
 RUN: printf '\0\0\0\0\0\0\0\2' >> %t
 RUN:

[PATCH] D104556: [InstrProfiling] Make CountersPtr in __profd_ relative

2021-07-03 Thread Fangrui Song via Phabricator via cfe-commits 
MaskRay updated this revision to Diff 356271.
MaskRay edited the summary of this revision.
MaskRay added a comment.

Drop version 5 compatibility because the Linux kernel patch is suspended.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104556

Files:
  clang/test/Profile/c-linkage-available_externally.c
  compiler-rt/include/profile/InstrProfData.inc
  compiler-rt/lib/profile/InstrProfilingMerge.c
  compiler-rt/lib/profile/InstrProfilingWriter.c
  llvm/include/llvm/ProfileData/InstrProfData.inc
  llvm/lib/ProfileData/InstrProfReader.cpp
  llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp
  llvm/test/Instrumentation/InstrProfiling/icall.ll
  llvm/test/Instrumentation/InstrProfiling/profiling.ll
  llvm/test/Transforms/PGOProfile/comdat_internal.ll
  llvm/test/Transforms/PGOProfile/indirect_call_profile.ll
  llvm/test/Transforms/PGOProfile/memcpy.ll
  llvm/test/tools/llvm-profdata/Inputs/c-general.profraw
  llvm/test/tools/llvm-profdata/c-general.test
  llvm/test/tools/llvm-profdata/malformed-ptr-to-counter-array.test
  llvm/test/tools/llvm-profdata/raw-32-bits-be.test
  llvm/test/tools/llvm-profdata/raw-32-bits-le.test
  llvm/test/tools/llvm-profdata/raw-64-bits-be.test
  llvm/test/tools/llvm-profdata/raw-64-bits-le.test
  llvm/test/tools/llvm-profdata/raw-two-profiles.test

Index: llvm/test/tools/llvm-profdata/raw-two-profiles.test
===
--- llvm/test/tools/llvm-profdata/raw-two-profiles.test
+++ llvm/test/tools/llvm-profdata/raw-two-profiles.test
@@ -1,5 +1,5 @@
 RUN: printf '\201rforpl\377' > %t-foo.profraw
-RUN: printf '\5\0\0\0\0\0\0\0' >> %t-foo.profraw
+RUN: printf '\6\0\0\0\0\0\0\0' >> %t-foo.profraw
 RUN: printf '\1\0\0\0\0\0\0\0' >> %t-foo.profraw
 RUN: printf '\0\0\0\0\0\0\0\0' >> %t-foo.profraw
 RUN: printf '\1\0\0\0\0\0\0\0' >> %t-foo.profraw
@@ -20,7 +20,7 @@
 RUN: printf '\3\0foo\0\0\0' >> %t-foo.profraw
 
 RUN: printf '\201rforpl\377' > %t-bar.profraw
-RUN: printf '\5\0\0\0\0\0\0\0' >> %t-bar.profraw
+RUN: printf '\6\0\0\0\0\0\0\0' >> %t-bar.profraw
 RUN: printf '\1\0\0\0\0\0\0\0' >> %t-bar.profraw
 RUN: printf '\0\0\0\0\0\0\0\0' >> %t-bar.profraw
 RUN: printf '\2\0\0\0\0\0\0\0' >> %t-bar.profraw
Index: llvm/test/tools/llvm-profdata/raw-64-bits-le.test
===
--- llvm/test/tools/llvm-profdata/raw-64-bits-le.test
+++ llvm/test/tools/llvm-profdata/raw-64-bits-le.test
@@ -1,5 +1,5 @@
 RUN: printf '\201rforpl\377' > %t
-RUN: printf '\5\0\0\0\0\0\0\0' >> %t
+RUN: printf '\6\0\0\0\0\0\0\0' >> %t
 RUN: printf '\2\0\0\0\0\0\0\0' >> %t
 RUN: printf '\0\0\0\0\0\0\0\0' >> %t
 RUN: printf '\3\0\0\0\0\0\0\0' >> %t
@@ -18,7 +18,7 @@
 
 RUN: printf '\067\265\035\031\112\165\023\344' >> %t
 RUN: printf '\02\0\0\0\0\0\0\0' >> %t
-RUN: printf '\10\0\4\0\1\0\0\0' >> %t
+RUN: printf '\xd8\xff\3\0\1\0\0\0' >> %t
 RUN: printf '\0\0\0\0\0\0\0\0' >> %t
 RUN: printf '\0\0\0\0\0\0\0\0' >> %t
 RUN: printf '\02\0\0\0\0\0\0\0' >> %t
Index: llvm/test/tools/llvm-profdata/raw-64-bits-be.test
===
--- llvm/test/tools/llvm-profdata/raw-64-bits-be.test
+++ llvm/test/tools/llvm-profdata/raw-64-bits-be.test
@@ -1,5 +1,5 @@
 RUN: printf '\377lprofr\201' > %t
-RUN: printf '\0\0\0\0\0\0\0\5' >> %t
+RUN: printf '\0\0\0\0\0\0\0\6' >> %t
 RUN: printf '\0\0\0\0\0\0\0\2' >> %t
 RUN: printf '\0\0\0\0\0\0\0\0' >> %t
 RUN: printf '\0\0\0\0\0\0\0\3' >> %t
@@ -18,7 +18,7 @@
 
 RUN: printf '\344\023\165\112\031\035\265\067' >> %t
 RUN: printf '\0\0\0\0\0\0\0\02' >> %t
-RUN: printf '\0\0\0\1\0\4\0\10' >> %t
+RUN: printf '\0\0\0\1\0\3\xff\xd8' >> %t
 RUN: printf '\0\0\0\0\0\0\0\0' >> %t
 RUN: printf '\0\0\0\0\0\0\0\0' >> %t
 RUN: printf '\0\0\0\02\0\0\0\0' >> %t
Index: llvm/test/tools/llvm-profdata/raw-32-bits-le.test
===
--- llvm/test/tools/llvm-profdata/raw-32-bits-le.test
+++ llvm/test/tools/llvm-profdata/raw-32-bits-le.test
@@ -1,5 +1,5 @@
 RUN: printf '\201Rforpl\377' > %t
-RUN: printf '\5\0\0\0\0\0\0\0' >> %t
+RUN: printf '\6\0\0\0\0\0\0\0' >> %t
 RUN: printf '\2\0\0\0\0\0\0\0' >> %t
 RUN: printf '\0\0\0\0\0\0\0\0' >> %t
 RUN: printf '\3\0\0\0\0\0\0\0' >> %t
@@ -19,7 +19,7 @@
 
 RUN: printf '\067\265\035\031\112\165\023\344' >> %t
 RUN: printf '\02\0\0\0\0\0\0\0' >> %t
-RUN: printf '\10\0\0\1' >> %t
+RUN: printf '\xe0\xff\xff\0' >> %t
 RUN: printf '\0\0\0\0' >> %t
 RUN: printf '\0\0\0\0' >> %t
 RUN: printf '\2\0\0\0' >> %t
Index: llvm/test/tools/llvm-profdata/raw-32-bits-be.test
===
--- llvm/test/tools/llvm-profdata/raw-32-bits-be.test
+++ llvm/test/tools/llvm-profdata/raw-32-bits-be.test
@@ -1,5 +1,5 @@
 RUN: printf '\377lprofR\201' > %t
-RUN: printf '\0\0\0\0\0\0\0\5' >> %t
+RUN: printf '\0\0\0\0\0\0\0\6' >> %t
 RUN: printf

[PATCH] D104556: [InstrProfiling] Make CountersPtr in __profd_ relative

2021-07-02 Thread Reid Kleckner via Phabricator via cfe-commits 
rnk accepted this revision.
rnk added a comment.
This revision is now accepted and ready to land.

I'm happy with this.




Comment at: compiler-rt/include/profile/InstrProfData.inc:79
+INSTR_PROF_DATA(const IntPtrT, IntPtrTy, CounterPtr,
+ConstantExpr::getSub(ConstantExpr::getPtrToInt(CounterPtr,
+   IntPtrTy),

I'd suggest moving this into a local variable in the instrumentation code. The 
less code there is in this header, the better.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104556

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

[PATCH] D104556: [InstrProfiling] Make CountersPtr in __profd_ relative

2021-06-28 Thread Fangrui Song via Phabricator via cfe-commits 
MaskRay added a comment.

Ping:)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104556

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

[PATCH] D104556: [InstrProfiling] Make CountersPtr in __profd_ relative

2021-06-24 Thread Fangrui Song via Phabricator via cfe-commits 
MaskRay added a comment.

In D104556#2837136 , @mstorsjo wrote:

> In D104556#2837104 , @rnk wrote:
>
>> Swift wanted the same thing, so I think the answer is yes, we should ask.
>
> What would the benefit of that be, as the difference itself can only ever be 
> 32 bit? Is it only for consistency with other binary formats? I guess getting 
> the value sign extended to a 64 bit value is a bit useful too.

Yes, consistency among binary formats. A 64-bit label difference `.quad A-B` is 
directly expressable on Mach-O and RISC-V (`R_RISCV_SUB*`) and can be converted 
to PC-relative on ELF.

For example, mips64 supports 64-bit PC-relative since D80390 
 (GNU as still doesn't allow it) => this 
allowed an XRay clean-up D87977 .


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104556

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

[PATCH] D104556: [InstrProfiling] Make CountersPtr in __profd_ relative

2021-06-23 Thread Martin Storsjö via Phabricator via cfe-commits 
mstorsjo added a comment.

In D104556#2837104 , @rnk wrote:

> In D104556#2837006 , @MaskRay wrote:
>
>> Is it possible to ask MSVC to add the 64-bit `IMAGE_REL_AMD64_REL64` and 
>> `IMAGE_REL_ARM64_REL64`?
>> Newer compiler metadata techniques can benefit from using the same mechanism 
>> for all three major binary formats (PE-COFF/ELF/Mach-O).
>
> Swift wanted the same thing, so I think the answer is yes, we should ask.

What would the benefit of that be, as the difference itself can only ever be 32 
bit? Is it only for consistency with other binary formats? I guess getting the 
value sign extended to a 64 bit value is a bit useful too.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104556

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

[PATCH] D104556: [InstrProfiling] Make CountersPtr in __profd_ relative

2021-06-23 Thread Reid Kleckner via Phabricator via cfe-commits 
rnk added a comment.

In D104556#2837006 , @MaskRay wrote:

> Is it possible to ask MSVC to add the 64-bit `IMAGE_REL_AMD64_REL64` and 
> `IMAGE_REL_ARM64_REL64`?
> Newer compiler metadata techniques can benefit from using the same mechanism 
> for all three major binary formats (PE-COFF/ELF/Mach-O).

Swift wanted the same thing, so I think the answer is yes, we should ask.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104556

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

[PATCH] D104556: [InstrProfiling] Make CountersPtr in __profd_ relative

2021-06-23 Thread Fangrui Song via Phabricator via cfe-commits 
MaskRay added a comment.

In D104556#2836900 , @rnk wrote:

> In D104556#2831787 , @MaskRay wrote:
>
>> Hmm, IMGREL (`IMAGE_REL_AMD64_ADDR32NB`) looks useful and is an alternative 
>> solution to PC-relative relocations in ELF / relocation subtraction in 
>> Mach-O.
>> But leveraging it seems to need more code in the runtime and will make COFF 
>> vs non-COFF different in IR, runtime, and llvm-profdata
>
> We can go forward with what we have, but the `signextIfWin64` helper is 
> pretty subtle. In some ways, I think adding `__ImageBase` is clearer:
>
>   #ifdef _WIN32
>   extern "C" char __ImageBase;
>   #endif
>   uintptr_t rebaseRelativePtr(void *D, void *P) {
>   #ifdef _WIN32
> return (uintptr_t)&__ImageBase + (uintptr_t)P;
>   #else
> return (uintptr_t)D + (uintptr_t)P;
>   #endif
>   }
>
> It's not quite this easy, and the instrumentation side changes have to 
> basically copy or refactor this code:
> https://github.com/llvm/llvm-project/blob/main/clang/lib/CodeGen/MicrosoftCXXABI.cpp#L548
>
> We aren't worried about profraw format compatibility on Windows, so I think 
> we can change this later at any time if we like.

I think you mean that `INSTR_PROF_RAW_HEADER(uint64_t, CountersDelta, 
(uintptr_t)CountersBegin - (uintptr_t)DataBegin)` (in InstrProfiling.cpp, so 
used by both -fprofile-generate and -fprofile-instr-generate) needs to adopt 
the scheme in 
https://github.com/llvm/llvm-project/blob/main/clang/lib/CodeGen/MicrosoftCXXABI.cpp#L548
Yes, seems quite a bit of complexity.

Is it possible to ask MSVC to add the 64-bit `IMAGE_REL_AMD64_REL64` and 
`IMAGE_REL_ARM64_REL64`?
Newer compiler metadata techniques can benefit from using the same mechanism 
for all three major binary formats (PE-COFF/ELF/Mach-O).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104556

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

[PATCH] D104556: [InstrProfiling] Make CountersPtr in __profd_ relative

2021-06-23 Thread Reid Kleckner via Phabricator via cfe-commits 
rnk added a comment.

In D104556#2831787 , @MaskRay wrote:

> Hmm, IMGREL (`IMAGE_REL_AMD64_ADDR32NB`) looks useful and is an alternative 
> solution to PC-relative relocations in ELF / relocation subtraction in Mach-O.
> But leveraging it seems to need more code in the runtime and will make COFF 
> vs non-COFF different in IR, runtime, and llvm-profdata

We can go forward with what we have, but the `signextIfWin64` helper is pretty 
subtle. In some ways, I think adding `__ImageBase` is clearer:

  #ifdef _WIN32
  extern "C" char __ImageBase;
  #endif
  uintptr_t rebaseRelativePtr(void *D, void *P) {
  #ifdef _WIN32
return (uintptr_t)&__ImageBase + (uintptr_t)P;
  #else
return (uintptr_t)D + (uintptr_t)P;
  #endif
  }

It's not quite this easy, and the instrumentation side changes have to 
basically copy or refactor this code:
https://github.com/llvm/llvm-project/blob/main/clang/lib/CodeGen/MicrosoftCXXABI.cpp#L548

We aren't worried about profraw format compatibility on Windows, so I think we 
can change this later at any time if we like.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104556

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

[PATCH] D104556: [InstrProfiling] Make CountersPtr in __profd_ relative

2021-06-23 Thread Vedant Kumar via Phabricator via cfe-commits 
vsk added a comment.

A one-time exception to the .profraw compatibility policy sounds reasonable to 
me.

A little more context: llvm has historically rev'd the .profraw format with 
abandon to deliver performance/size improvements (as David & co. did with name 
compression) and new functionality (value profiling, continuous sync mode, 
...). That will keep happening. The freedom to rev the raw format comes from 
keeping it private between the profile reader library and the runtime. Some 
alternatives to hardcoding details of a particular version of the .profraw 
format were mentioned earlier in the thread: if the Linux PGO support can 
pursue one of these (or find some other solution), I think it will it prevent 
headaches down the road.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104556

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

[PATCH] D104556: [InstrProfiling] Make CountersPtr in __profd_ relative

2021-06-21 Thread Fangrui Song via Phabricator via cfe-commits 
MaskRay updated this revision to Diff 353519.
MaskRay edited the summary of this revision.
MaskRay added a comment.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Mention the one-time exception for .profraw compatibility


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104556

Files:
  clang/test/Profile/c-linkage-available_externally.c
  compiler-rt/include/profile/InstrProfData.inc
  compiler-rt/lib/profile/InstrProfilingMerge.c
  compiler-rt/lib/profile/InstrProfilingWriter.c
  llvm/include/llvm/ProfileData/InstrProfData.inc
  llvm/include/llvm/ProfileData/InstrProfReader.h
  llvm/lib/ProfileData/InstrProfReader.cpp
  llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp
  llvm/test/Instrumentation/InstrProfiling/icall.ll
  llvm/test/Instrumentation/InstrProfiling/profiling.ll
  llvm/test/Transforms/PGOProfile/comdat_internal.ll
  llvm/test/Transforms/PGOProfile/indirect_call_profile.ll
  llvm/test/Transforms/PGOProfile/memcpy.ll
  llvm/test/tools/llvm-profdata/Inputs/c-general.profraw
  llvm/test/tools/llvm-profdata/c-general.test
  llvm/test/tools/llvm-profdata/malformed-ptr-to-counter-array.test
  llvm/test/tools/llvm-profdata/raw-32-bits-be.test
  llvm/test/tools/llvm-profdata/raw-32-bits-le.test
  llvm/test/tools/llvm-profdata/raw-64-bits-be.test
  llvm/test/tools/llvm-profdata/raw-64-bits-le-v5.test
  llvm/test/tools/llvm-profdata/raw-64-bits-le.test
  llvm/test/tools/llvm-profdata/raw-two-profiles.test

Index: llvm/test/tools/llvm-profdata/raw-two-profiles.test
===
--- llvm/test/tools/llvm-profdata/raw-two-profiles.test
+++ llvm/test/tools/llvm-profdata/raw-two-profiles.test
@@ -1,5 +1,5 @@
 RUN: printf '\201rforpl\377' > %t-foo.profraw
-RUN: printf '\5\0\0\0\0\0\0\0' >> %t-foo.profraw
+RUN: printf '\6\0\0\0\0\0\0\0' >> %t-foo.profraw
 RUN: printf '\1\0\0\0\0\0\0\0' >> %t-foo.profraw
 RUN: printf '\0\0\0\0\0\0\0\0' >> %t-foo.profraw
 RUN: printf '\1\0\0\0\0\0\0\0' >> %t-foo.profraw
@@ -20,7 +20,7 @@
 RUN: printf '\3\0foo\0\0\0' >> %t-foo.profraw
 
 RUN: printf '\201rforpl\377' > %t-bar.profraw
-RUN: printf '\5\0\0\0\0\0\0\0' >> %t-bar.profraw
+RUN: printf '\6\0\0\0\0\0\0\0' >> %t-bar.profraw
 RUN: printf '\1\0\0\0\0\0\0\0' >> %t-bar.profraw
 RUN: printf '\0\0\0\0\0\0\0\0' >> %t-bar.profraw
 RUN: printf '\2\0\0\0\0\0\0\0' >> %t-bar.profraw
Index: llvm/test/tools/llvm-profdata/raw-64-bits-le.test
===
--- llvm/test/tools/llvm-profdata/raw-64-bits-le.test
+++ llvm/test/tools/llvm-profdata/raw-64-bits-le.test
@@ -1,5 +1,5 @@
 RUN: printf '\201rforpl\377' > %t
-RUN: printf '\5\0\0\0\0\0\0\0' >> %t
+RUN: printf '\6\0\0\0\0\0\0\0' >> %t
 RUN: printf '\2\0\0\0\0\0\0\0' >> %t
 RUN: printf '\0\0\0\0\0\0\0\0' >> %t
 RUN: printf '\3\0\0\0\0\0\0\0' >> %t
@@ -18,7 +18,7 @@
 
 RUN: printf '\067\265\035\031\112\165\023\344' >> %t
 RUN: printf '\02\0\0\0\0\0\0\0' >> %t
-RUN: printf '\10\0\4\0\1\0\0\0' >> %t
+RUN: printf '\xd8\xff\3\0\1\0\0\0' >> %t
 RUN: printf '\0\0\0\0\0\0\0\0' >> %t
 RUN: printf '\0\0\0\0\0\0\0\0' >> %t
 RUN: printf '\02\0\0\0\0\0\0\0' >> %t
Index: llvm/test/tools/llvm-profdata/raw-64-bits-le-v5.test
===
--- llvm/test/tools/llvm-profdata/raw-64-bits-le-v5.test
+++ llvm/test/tools/llvm-profdata/raw-64-bits-le-v5.test
@@ -1,3 +1,6 @@
+Test we can read the 64-bit version 5 raw profile format.
+TODO Drop the support after 13.0.0 is branched.
+
 RUN: printf '\201rforpl\377' > %t
 RUN: printf '\5\0\0\0\0\0\0\0' >> %t
 RUN: printf '\2\0\0\0\0\0\0\0' >> %t
Index: llvm/test/tools/llvm-profdata/raw-64-bits-be.test
===
--- llvm/test/tools/llvm-profdata/raw-64-bits-be.test
+++ llvm/test/tools/llvm-profdata/raw-64-bits-be.test
@@ -1,5 +1,5 @@
 RUN: printf '\377lprofr\201' > %t
-RUN: printf '\0\0\0\0\0\0\0\5' >> %t
+RUN: printf '\0\0\0\0\0\0\0\6' >> %t
 RUN: printf '\0\0\0\0\0\0\0\2' >> %t
 RUN: printf '\0\0\0\0\0\0\0\0' >> %t
 RUN: printf '\0\0\0\0\0\0\0\3' >> %t
@@ -18,7 +18,7 @@
 
 RUN: printf '\344\023\165\112\031\035\265\067' >> %t
 RUN: printf '\0\0\0\0\0\0\0\02' >> %t
-RUN: printf '\0\0\0\1\0\4\0\10' >> %t
+RUN: printf '\0\0\0\1\0\3\xff\xd8' >> %t
 RUN: printf '\0\0\0\0\0\0\0\0' >> %t
 RUN: printf '\0\0\0\0\0\0\0\0' >> %t
 RUN: printf '\0\0\0\02\0\0\0\0' >> %t
Index: llvm/test/tools/llvm-profdata/raw-32-bits-le.test
===
--- llvm/test/tools/llvm-profdata/raw-32-bits-le.test
+++ llvm/test/tools/llvm-profdata/raw-32-bits-le.test
@@ -1,5 +1,5 @@
 RUN: printf '\201Rforpl\377' > %t
-RUN: printf '\5\0\0\0\0\0\0\0' >> %t
+RUN: printf '\6\0\0\0\0\0\0\0' >> %t
 RUN: printf '\2\0\0\0\0\0\0\0' >> %t
 RUN: printf '\0\0\0\0\0\0\0\0' >> %t
 RUN: printf '\3\0\0\0\0\0\0\0' >> %t
@@ -19,7 +19,7