Page MenuHomePhabricator

MaskRay (Fangrui Song)
UserAdministrator

Projects

User Details

User Since
Dec 30 2016, 3:24 PM (233 w, 3 d)
Roles
Administrator

Recent Activity

Today

MaskRay updated the diff for D104556: [InstrProfiling] Make CountersPtr in __profd_ relative.

Mention the one-time exception for .profraw compatibility

Mon, Jun 21, 5:57 PM · Restricted Project, Restricted Project, Restricted Project
MaskRay added a comment to D104556: [InstrProfiling] Make CountersPtr in __profd_ relative.

Not keeping version compatibility for the raw profile format would actually make my life easier.
But I was CCed on the Linux kernel news https://www.phoronix.com/scan.php?page=news_item&px=Clang-PGO-For-Linux-Next so I paid additional care in this patch.

Mon, Jun 21, 4:53 PM · Restricted Project, Restricted Project, Restricted Project
MaskRay updated the diff for D104667: Improve the diagnostic of DiagnosticInfoResourceLimit (and warn-stack-size in particular).

rebase

Mon, Jun 21, 4:15 PM · Restricted Project, Restricted Project
MaskRay added inline comments to D104667: Improve the diagnostic of DiagnosticInfoResourceLimit (and warn-stack-size in particular).
Mon, Jun 21, 3:55 PM · Restricted Project, Restricted Project
MaskRay added a comment to D104556: [InstrProfiling] Make CountersPtr in __profd_ relative.

On COFF, it would be more traditional to make these image-relative 32-bit offsets, so they would come out as .long __profc_foo@IMGREL. If you grep for IMGREL in test/, you can see various exception handling and RTTI tables use that structure.

I seem to recall this was also an issue for swift, which wanted label difference relocations.

The absolute function address (used by llvm-profdata to collect indirect call
targets) can be converted to relative as well, but is not done in this patch.

If we're doing a format change, we should think about this a bit. Previously I was considering removing this field altogether and trying to store this information in a new section, but I don't think it's worth it after D102818.

Mon, Jun 21, 3:46 PM · Restricted Project, Restricted Project, Restricted Project
MaskRay updated the diff for D104556: [InstrProfiling] Make CountersPtr in __profd_ relative.

Add more comments to InstrProfilingMerge.c

Mon, Jun 21, 3:20 PM · Restricted Project, Restricted Project, Restricted Project
MaskRay added a comment to D104564: [AArch64][X86] Allow 64-bit label differences lower to IMAGE_REL_*_REL32.

I was going to say, I wonder if it would be better to find a way to put the extension in IR instead of in the assembler. For example, maybe the IR could look like (pseudo-IR):
zext i64 (trunc i32 (sub i64 (ptrtoint i64 @label2)), (ptrtoint i64 @label1))))
So that would slot into the constant inititalizer, and the backend would know to emit that pattern as:

.long 0
.long label2 - label1

Now that I've written it out, it seems worse and more complex. Let's go with .quad I guess.

Mon, Jun 21, 2:49 PM · Restricted Project
MaskRay updated the summary of D104667: Improve the diagnostic of DiagnosticInfoResourceLimit (and warn-stack-size in particular).
Mon, Jun 21, 2:45 PM · Restricted Project, Restricted Project
MaskRay updated the diff for D104667: Improve the diagnostic of DiagnosticInfoResourceLimit (and warn-stack-size in particular).

stack size -> stack frame size

Mon, Jun 21, 2:45 PM · Restricted Project, Restricted Project
MaskRay retitled D104667: Improve the diagnostic of DiagnosticInfoResourceLimit (and warn-stack-size in particular) from Improve the diagnostic of DiagnosticInfoStackSize (and warn-stack-size in particular) to Improve the diagnostic of DiagnosticInfoResourceLimit (and warn-stack-size in particular).
Mon, Jun 21, 2:39 PM · Restricted Project, Restricted Project
MaskRay committed rGc618692218d1: [AArch64][X86] Allow 64-bit label differences lower to IMAGE_REL_*_REL32 (authored by MaskRay).
[AArch64][X86] Allow 64-bit label differences lower to IMAGE_REL_*_REL32
Mon, Jun 21, 2:32 PM
MaskRay closed D104564: [AArch64][X86] Allow 64-bit label differences lower to IMAGE_REL_*_REL32.
Mon, Jun 21, 2:32 PM · Restricted Project
MaskRay added a comment to D104060: Machine IR Profile.

__llvm_mipmap section should have type ELF::SHF_NOTE so that it doesn't get stripped by --gc-sections.

Mon, Jun 21, 2:28 PM · Restricted Project
MaskRay added a comment to D104667: Improve the diagnostic of DiagnosticInfoResourceLimit (and warn-stack-size in particular).

GCC trunk: warning: the frame size of 80 bytes is larger than 0 bytes [-Wframe-larger-than=]
Clang trunk: warning: stack frame size of 88 bytes in function 'bar' [-Wframe-larger-than]

looks like the frontend doesn't use this particular text at all?

This diagnostic is emitted by clang (clang/lib/CodeGen/CodeGenAction.cpp).

-Wframe-larger-than= also records the module flag metadata "warn-stack-size" which can be useful for LTO.

% fclang -Wframe-larger-than=100 a.c  -o x          
a.c:1:5: warning: stack frame size of 888 bytes in function 'main' [-Wframe-larger-than]
int main(){
    ^
1 warning generated.
% fclang -Wframe-larger-than=100 -flto=thin -fuse-ld=lld a.c -o x
ld.lld: warning: stack size limit exceeded (888) in main

Can we try to make the FE match what's printed during LTO? It's odd that they differ, IMO. Also, GCC prints the threshold value, as does our backend; but the frontend still does not.

Mon, Jun 21, 1:58 PM · Restricted Project, Restricted Project
MaskRay added a comment to D104342: [IR] convert warn-stack-size from module flag to fn attr.

module attr

Mon, Jun 21, 1:53 PM · Restricted Project, Restricted Project
MaskRay accepted D104342: [IR] convert warn-stack-size from module flag to fn attr.

Agree that a function attribute is more appropriate considering the LTO behavior.

Mon, Jun 21, 1:51 PM · Restricted Project, Restricted Project
MaskRay added a comment to D104667: Improve the diagnostic of DiagnosticInfoResourceLimit (and warn-stack-size in particular).

GCC trunk: warning: the frame size of 80 bytes is larger than 0 bytes [-Wframe-larger-than=]
Clang trunk: warning: stack frame size of 88 bytes in function 'bar' [-Wframe-larger-than]

looks like the frontend doesn't use this particular text at all?

Mon, Jun 21, 1:25 PM · Restricted Project, Restricted Project
MaskRay requested review of D104667: Improve the diagnostic of DiagnosticInfoResourceLimit (and warn-stack-size in particular).
Mon, Jun 21, 1:06 PM · Restricted Project, Restricted Project
MaskRay accepted D104541: [Utils][vim] Add missing highlights for fast-math flags.

Looks great!

Mon, Jun 21, 12:04 PM · Restricted Project
MaskRay accepted D100734: Fix SpecCPU2017's dependency from timeit-target to build-timeit-target.

LGTM.

Mon, Jun 21, 12:03 PM
MaskRay accepted D104566: [UpdateCCTestChecks] Fix --replace-value-regex across RUN lines.

LG

Mon, Jun 21, 12:02 PM · Restricted Project, Restricted Project
MaskRay committed rGea23c38d0605: [llvm-profdata] Allow omission of -o for --text output (authored by MaskRay).
[llvm-profdata] Allow omission of -o for --text output
Mon, Jun 21, 12:02 PM
MaskRay closed D104600: [llvm-profdata] Allow omission of -o for --text output.
Mon, Jun 21, 12:02 PM · Restricted Project
MaskRay accepted D104658: [Clang][Codegen] rename no_profile fn attr no_profile_instrument_function.

(This was why I suggested we waited a bit on GCC's response...)

Mon, Jun 21, 11:20 AM · Restricted Project

Yesterday

MaskRay committed rG89e66a3ab3b2: [ELF] Delete --no-cref which does not exist in GNU ld (authored by MaskRay).
[ELF] Delete --no-cref which does not exist in GNU ld
Sun, Jun 20, 2:29 PM
MaskRay committed rGcd6b1b2b865a: [ELF][test] Add missing tests for --no-export-dynamic & --no-warn-backrefs (authored by MaskRay).
[ELF][test] Add missing tests for --no-export-dynamic & --no-warn-backrefs
Sun, Jun 20, 2:20 PM
MaskRay committed rG558ee5843f9f: [mlir] Fix -Wunused-but-set-variable in -DLLVM_ENABLE_ASSERTIONS=off build. NFC (authored by MaskRay).
[mlir] Fix -Wunused-but-set-variable in -DLLVM_ENABLE_ASSERTIONS=off build. NFC
Sun, Jun 20, 11:55 AM
MaskRay committed rG50225112b56a: [lld-link] Fix -Wunused-but-set-variable in -DLLVM_ENABLE_ASSERTIONS=off build. (authored by MaskRay).
[lld-link] Fix -Wunused-but-set-variable in -DLLVM_ENABLE_ASSERTIONS=off build.
Sun, Jun 20, 11:35 AM
MaskRay committed rG521d37327422: Fix -Wunused-variable and -Wunused-but-set-variable in… (authored by MaskRay).
Fix -Wunused-variable and -Wunused-but-set-variable in…
Sun, Jun 20, 11:09 AM

Sat, Jun 19

MaskRay committed rG0873016ceff3: [llvm-cov gcov] Support GCC 12 format (authored by MaskRay).
[llvm-cov gcov] Support GCC 12 format
Sat, Jun 19, 10:51 PM
MaskRay committed rGe85eecff3068: [llvm-cov gcov] Change case to match the prevailing style && replace getString… (authored by MaskRay).
[llvm-cov gcov] Change case to match the prevailing style && replace getString…
Sat, Jun 19, 10:51 PM
MaskRay committed rGcee85fcd766c: [test] Fix nocompress.test (authored by MaskRay).
[test] Fix nocompress.test
Sat, Jun 19, 4:28 PM
MaskRay updated the summary of D104600: [llvm-profdata] Allow omission of -o for --text output.
Sat, Jun 19, 4:14 PM · Restricted Project
MaskRay requested review of D104600: [llvm-profdata] Allow omission of -o for --text output.
Sat, Jun 19, 4:13 PM · Restricted Project
MaskRay committed rG8ea2a58a2ec6: [llvm-profdata] Make diagnostics consistent with the (no capitalization, no… (authored by MaskRay).
[llvm-profdata] Make diagnostics consistent with the (no capitalization, no…
Sat, Jun 19, 2:54 PM
MaskRay committed rG0f558db742fa: [llvm-profdata] Delete unneeded empty output filename check (authored by MaskRay).
[llvm-profdata] Delete unneeded empty output filename check
Sat, Jun 19, 12:21 PM
MaskRay committed rG59d90fe817b5: Simplify some typedef struct (authored by MaskRay).
Simplify some typedef struct
Sat, Jun 19, 11:37 AM

Fri, Jun 18

MaskRay added reviewers for D104556: [InstrProfiling] Make CountersPtr in __profd_ relative: davidxl, rnk, vsk.
Fri, Jun 18, 11:17 PM · Restricted Project, Restricted Project, Restricted Project
MaskRay updated subscribers of D104556: [InstrProfiling] Make CountersPtr in __profd_ relative.
Fri, Jun 18, 10:28 PM · Restricted Project, Restricted Project, Restricted Project
MaskRay updated the diff for D104556: [InstrProfiling] Make CountersPtr in __profd_ relative.

make win64 work

Fri, Jun 18, 10:26 PM · Restricted Project, Restricted Project, Restricted Project
MaskRay added a comment to D104060: Machine IR Profile.

I probably sound like a broken record, but we've spent a lot of time making sure the map section can be extracted correctly and that the raw section has no excess info. This is a major feature of MIP, the profile data and the function info are separated so that only the necessary data remains in the binary. The question is whether we should extend an existing pgi to support this feature or if MIP deserves to be its own framework. I do see the value in extending one of the many existing pgi to reduce duplicate work. My thoughts are that it would be too invasive to do everything we need; add one or two new sections, create a new .profraw and .profmap format, add a few flags, and extend the tools. By keeping MIP separate, we can make design decisions that align with our code size goal that may not be so easy to do in existing frameworks.

Fri, Jun 18, 8:16 PM · Restricted Project
MaskRay updated the summary of D104556: [InstrProfiling] Make CountersPtr in __profd_ relative.
Fri, Jun 18, 6:12 PM · Restricted Project, Restricted Project, Restricted Project
MaskRay updated the diff for D104556: [InstrProfiling] Make CountersPtr in __profd_ relative.

update

Fri, Jun 18, 6:12 PM · Restricted Project, Restricted Project, Restricted Project
MaskRay updated the summary of D104556: [InstrProfiling] Make CountersPtr in __profd_ relative.
Fri, Jun 18, 5:52 PM · Restricted Project, Restricted Project, Restricted Project
MaskRay committed rG3307240f057b: [InstrProfiling][ELF] Make __profd_ private if the function does not use value… (authored by MaskRay).
[InstrProfiling][ELF] Make __profd_ private if the function does not use value…
Fri, Jun 18, 5:01 PM
MaskRay closed D103717: [InstrProfiling][ELF] Make __profd_ private if the function does not use value profiling.
Fri, Jun 18, 5:01 PM · Restricted Project
MaskRay committed rG5540470f642a: [profile][test] Delete profraw directory so that tests are immune to format… (authored by MaskRay).
[profile][test] Delete profraw directory so that tests are immune to format…
Fri, Jun 18, 4:44 PM
MaskRay added a comment to D103717: [InstrProfiling][ELF] Make __profd_ private if the function does not use value profiling.

Thanks for review:) I feel that I understand value profiling better now.

Fri, Jun 18, 4:18 PM · Restricted Project
MaskRay updated the diff for D103717: [InstrProfiling][ELF] Make __profd_ private if the function does not use value profiling.

minor adjustment to comment and test

Fri, Jun 18, 4:12 PM · Restricted Project
MaskRay updated the diff for D103717: [InstrProfiling][ELF] Make __profd_ private if the function does not use value profiling.

switch to the NS==0 approach after seeing See my prior comment -- your previous revision is sound and LGTM. If we go with this version, we do need to examine whether static allocation is on or not. Sorry for the trouble.
(did not notice in my previous test update...)

Fri, Jun 18, 4:04 PM · Restricted Project
MaskRay updated the diff for D103717: [InstrProfiling][ELF] Make __profd_ private if the function does not use value profiling.

fix condition and improve test

Fri, Jun 18, 3:57 PM · Restricted Project
MaskRay updated the diff for D103717: [InstrProfiling][ELF] Make __profd_ private if the function does not use value profiling.

Use ValuesVar instead of NS

Fri, Jun 18, 3:32 PM · Restricted Project
MaskRay added inline comments to D103717: [InstrProfiling][ELF] Make __profd_ private if the function does not use value profiling.
Fri, Jun 18, 3:11 PM · Restricted Project
MaskRay added inline comments to D103717: [InstrProfiling][ELF] Make __profd_ private if the function does not use value profiling.
Fri, Jun 18, 2:40 PM · Restricted Project
MaskRay updated the diff for D103717: [InstrProfiling][ELF] Make __profd_ private if the function does not use value profiling.

improve a comment

Fri, Jun 18, 1:58 PM · Restricted Project
MaskRay added inline comments to D103717: [InstrProfiling][ELF] Make __profd_ private if the function does not use value profiling.
Fri, Jun 18, 1:46 PM · Restricted Project
MaskRay updated the summary of D103717: [InstrProfiling][ELF] Make __profd_ private if the function does not use value profiling.
Fri, Jun 18, 1:42 PM · Restricted Project
MaskRay updated the diff for D103717: [InstrProfiling][ELF] Make __profd_ private if the function does not use value profiling.

add test

Fri, Jun 18, 1:41 PM · Restricted Project
MaskRay added a comment to D104564: [AArch64][X86] Allow 64-bit label differences lower to IMAGE_REL_*_REL32.

I think the main problem is when the value is negative. How to represent it?

Fri, Jun 18, 12:38 PM · Restricted Project
MaskRay added a comment to D104257: [GCOVProfiling] don't profile Fn's w/ noprofile attribute.

Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>

Fri, Jun 18, 12:17 PM · Restricted Project
MaskRay accepted D104257: [GCOVProfiling] don't profile Fn's w/ noprofile attribute.
Fri, Jun 18, 12:16 PM · Restricted Project
MaskRay added inline comments to D104475: [Clang][Codegen] Add GNU function attribute 'no_profile' and lower it to noprofile.
Fri, Jun 18, 12:10 PM · Restricted Project
MaskRay accepted D104475: [Clang][Codegen] Add GNU function attribute 'no_profile' and lower it to noprofile.

@marxin FYI the GNU-style function attribute no_profile

Fri, Jun 18, 12:09 PM · Restricted Project
MaskRay added a comment to D104475: [Clang][Codegen] Add GNU function attribute 'no_profile' and lower it to noprofile.

So if we don't want to offer guarantee for IR/C/C++ attributes, we can document that users may need to additionally specify __attribute__((noinline)) to suppress inlining.

I don't generally like that approach:

If a guarantee would cause inferior optimization or we cannot think of all the complication/fallout initially, I'd prefer that we keep the initial semantics narrow.

The guarantee is more important than the optimization.

If we cannot make a promise, don't provide a promise.
No-suppressing inlining makes the attribute freely composable with other attributes (https://lists.llvm.org/pipermail/llvm-dev/2021-April/150062.html )

IIUC, isn't @jdoerfert arguing that optnone and noinline being composable is _bad_?

Fri, Jun 18, 12:04 PM · Restricted Project
MaskRay requested review of D104564: [AArch64][X86] Allow 64-bit label differences lower to IMAGE_REL_*_REL32.
Fri, Jun 18, 11:56 AM · Restricted Project
MaskRay requested review of D104556: [InstrProfiling] Make CountersPtr in __profd_ relative.
Fri, Jun 18, 11:05 AM · Restricted Project, Restricted Project, Restricted Project
MaskRay added a comment to D104060: Machine IR Profile.

The main challenge is storing the offset to the profile data without using dynamic relocations. This is complicated by the fact that we use comdat sections within the llvm_mipraw section and that ELF does not seem to directly support section relative addresses. The solution is to use PC relative relocations. start___llvm_mipraw-.Lref gives us the PC relative offset to the start of the raw section and _Z3foov$RAW-.Lref gives us the offset to the profile data for this function relative to the same PC. After we extract the map section, we can subtract these to get the value we want, the section relative raw profile data offset.

(_Z3foov$RAW-.Lref) - (start_llvm_mipraw-.Lref) = _Z3foov$RAW - start_llvm_mipraw

Fri, Jun 18, 10:06 AM · Restricted Project
MaskRay added a comment to D104060: Machine IR Profile.

Some email conversations are not on Phabricator. I record a copy here so that people who are not subscribed can have a full view

Fri, Jun 18, 9:40 AM · Restricted Project
MaskRay added a comment to D102985: [lld][ELF][SPARC] Implement and fixup GOT/PLT relocations.

Is dynamic linking fully functional?

Fri, Jun 18, 9:23 AM · Restricted Project
MaskRay added a comment to D103717: [InstrProfiling][ELF] Make __profd_ private if the function does not use value profiling.

A stage-2 -DLLVM_BUILD_INSTRMENTED=IR clang is 4.2% smaller (1-169620032/177066968) with this patch.

Fri, Jun 18, 12:33 AM · Restricted Project
MaskRay requested review of D103717: [InstrProfiling][ELF] Make __profd_ private if the function does not use value profiling.
Fri, Jun 18, 12:31 AM · Restricted Project
MaskRay updated the diff for D103717: [InstrProfiling][ELF] Make __profd_ private if the function does not use value profiling.

I need to add a test case, but this is the sketch.

Fri, Jun 18, 12:31 AM · Restricted Project
MaskRay reopened D103717: [InstrProfiling][ELF] Make __profd_ private if the function does not use value profiling.
Fri, Jun 18, 12:26 AM · Restricted Project

Thu, Jun 17

MaskRay added a comment to D103717: [InstrProfiling][ELF] Make __profd_ private if the function does not use value profiling.

This patch breaks a two stage instrumented build::

$ cmake \
    -B build/stage1 \
    -G Ninja \
    -Wno-dev \
    -DCMAKE_BUILD_TYPE=Release \
    -DCMAKE_C_COMPILER=clang \
    -DCMAKE_CXX_COMPILER=clang++ \
    -DLLVM_CCACHE_BUILD=ON \
    -DLLVM_ENABLE_PROJECTS="clang;compiler-rt;lld" \
    -DLLVM_TARGETS_TO_BUILD=host \
    -DLLVM_USE_LINKER=lld \
    llvm

$ ninja -C build/stage1

$ export PATH=$PWD/build/stage1/bin:$PATH

$ cmake \
    -B build/stage2 \
    -G Ninja \
    -Wno-dev \
    -DCLANG_TABLEGEN=$(command -v clang-tblgen) \
    -DCMAKE_AR=$(command -v llvm-ar) \
    -DCMAKE_BUILD_TYPE=Release \
    -DCMAKE_C_COMPILER=$(command -v clang) \
    -DCMAKE_CXX_COMPILER=$(command -v clang++) \
    -DCMAKE_RANLIB=$(command -v llvm-ranlib) \
    -DLLVM_BUILD_INSTRUMENTED=IR \
    -DLLVM_BUILD_RUNTIME=OFF \
    -DLLVM_ENABLE_ASSERTIONS=ON \
    -DLLVM_ENABLE_PROJECTS="clang;lld" \
    -DLLVM_ENABLE_TERMINFO=OFF \
    -DLLVM_ENABLE_WARNINGS=OFF \
    -DLLVM_TABLEGEN=$(command -v llvm-tblgen) \
    -DLLVM_TARGETS_TO_BUILD=host \
    -DLLVM_USE_LINKER=$(command -v ld.lld) \
    llvm

$ ninja -C build/stage2
...
ld.lld: error: relocation refers to a discarded section: __llvm_prf_data
>>> defined in utils/TableGen/CMakeFiles/llvm-tblgen.dir/AsmWriterEmitter.cpp.o
>>> referenced by AsmWriterEmitter.cpp
>>>               utils/TableGen/CMakeFiles/llvm-tblgen.dir/AsmWriterEmitter.cpp.o:((anonymous namespace)::AsmWriterEmitter::run(llvm::raw_ostream&))
>>> referenced by AsmWriterEmitter.cpp
>>>               utils/TableGen/CMakeFiles/llvm-tblgen.dir/AsmWriterEmitter.cpp.o:((anonymous namespace)::AsmWriterEmitter::run(llvm::raw_ostream&))
>>> referenced by AsmWriterEmitter.cpp
>>>               utils/TableGen/CMakeFiles/llvm-tblgen.dir/AsmWriterEmitter.cpp.o:((anonymous namespace)::AsmWriterEmitter::run(llvm::raw_ostream&))
>>> referenced 232 more times

ld.lld: error: relocation refers to a discarded section: __llvm_prf_data
>>> defined in utils/TableGen/CMakeFiles/llvm-tblgen.dir/AsmWriterEmitter.cpp.o
>>> referenced by AsmWriterEmitter.cpp
>>>               utils/TableGen/CMakeFiles/llvm-tblgen.dir/AsmWriterEmitter.cpp.o:((anonymous namespace)::AsmWriterEmitter::run(llvm::raw_ostream&))
>>> referenced by AsmWriterEmitter.cpp
>>>               utils/TableGen/CMakeFiles/llvm-tblgen.dir/AsmWriterEmitter.cpp.o:((anonymous namespace)::AsmWriterEmitter::run(llvm::raw_ostream&))
>>> referenced by AsmWriterEmitter.cpp
>>>               utils/TableGen/CMakeFiles/llvm-tblgen.dir/AsmWriterEmitter.cpp.o:((anonymous namespace)::AsmWriterEmitter::run(llvm::raw_ostream&))
>>> referenced 8 more times
...
Thu, Jun 17, 11:40 PM · Restricted Project
MaskRay added a reverting change for rG76d0747e0807: [InstrProfiling] Make __profd_ unconditionally private for ELF: rG5798be84580b: Revert D103717 "[InstrProfiling] Make __profd_ unconditionally private for ELF".
Thu, Jun 17, 11:38 PM
MaskRay committed rG5798be84580b: Revert D103717 "[InstrProfiling] Make __profd_ unconditionally private for ELF" (authored by MaskRay).
Revert D103717 "[InstrProfiling] Make __profd_ unconditionally private for ELF"
Thu, Jun 17, 11:38 PM
MaskRay added a reverting change for D103717: [InstrProfiling][ELF] Make __profd_ private if the function does not use value profiling: rG5798be84580b: Revert D103717 "[InstrProfiling] Make __profd_ unconditionally private for ELF".
Thu, Jun 17, 11:38 PM · Restricted Project
MaskRay added a comment to D104475: [Clang][Codegen] Add GNU function attribute 'no_profile' and lower it to noprofile.

So if we don't want to offer guarantee for IR/C/C++ attributes, we can document that users may need to additionally specify __attribute__((noinline)) to suppress inlining.

I don't generally like that approach:

Thu, Jun 17, 3:26 PM · Restricted Project
MaskRay added a reviewer for D104475: [Clang][Codegen] Add GNU function attribute 'no_profile' and lower it to noprofile: jdoerfert.
Thu, Jun 17, 3:18 PM · Restricted Project
MaskRay added a comment to D104475: [Clang][Codegen] Add GNU function attribute 'no_profile' and lower it to noprofile.

Should no_profile specify the inlining behavior? Though no_sanitize_* don't specify that, either.

I think it's somehow implicit, but I can't quite say how. There are some tests that check this, e.g.
compiler-rt/test/asan/TestCases/inline.cpp
[...]
noinstr does add noinline, but other uses of the attribute alone might not. But in the end inlining is a red herring, it just seems to be the easiest way to enforce the promised contract. See https://lore.kernel.org/lkml/CANpmjNNRz5OVKb6PE7K6GjfoGbht_ZhyPkNG9aD+KjNDzK7hGg@mail.gmail.com/
[...]

Thu, Jun 17, 2:35 PM · Restricted Project
MaskRay committed rG76d0747e0807: [InstrProfiling] Make __profd_ unconditionally private for ELF (authored by MaskRay).
[InstrProfiling] Make __profd_ unconditionally private for ELF
Thu, Jun 17, 2:17 PM
MaskRay closed D103717: [InstrProfiling][ELF] Make __profd_ private if the function does not use value profiling.
Thu, Jun 17, 2:17 PM · Restricted Project
MaskRay added inline comments to D103717: [InstrProfiling][ELF] Make __profd_ private if the function does not use value profiling.
Thu, Jun 17, 1:42 PM · Restricted Project
MaskRay added a comment to D103717: [InstrProfiling][ELF] Make __profd_ private if the function does not use value profiling.

I think the previous Instrumentation/InstrProfiling.cpp changes can be considered stable now. So, ping:)

Thu, Jun 17, 1:30 PM · Restricted Project
MaskRay added a comment to D104475: [Clang][Codegen] Add GNU function attribute 'no_profile' and lower it to noprofile.

Should no_profile specify the inlining behavior? Though no_sanitize_* don't specify that, either.

Thu, Jun 17, 11:28 AM · Restricted Project
MaskRay accepted D104473: RISCV: clean up target expression handling.

Looks great! One nit

Thu, Jun 17, 11:05 AM · Restricted Project
MaskRay added inline comments to D104473: RISCV: clean up target expression handling.
Thu, Jun 17, 10:24 AM · Restricted Project

Wed, Jun 16

MaskRay added a comment to D104060: Machine IR Profile.

I think people's main question is what distinguishing features make MachineFunction instrumentation appealing.

Wed, Jun 16, 5:32 PM · Restricted Project
MaskRay committed rG7cfb7a67c57d: [mlir] Make Type::print and Type::dump const (authored by Robert David <lrdx@google.com>).
[mlir] Make Type::print and Type::dump const
Wed, Jun 16, 3:31 PM
MaskRay accepted D104353: [lld-macho] Avoid force-loading the same archive twice.
Wed, Jun 16, 2:29 PM · Restricted Project, Restricted Project
MaskRay added a comment to D102052: [MC][RISCV] Add RISCV MCObjectFileInfo.

riscv64-linux-gnu-as -march=rv64gc a.s does use 2-byte alignment, so I think this may be fine.

Wed, Jun 16, 1:42 PM · Restricted Project
MaskRay added reviewers for D102052: [MC][RISCV] Add RISCV MCObjectFileInfo: jrtc27, luismarques.
Wed, Jun 16, 1:41 PM · Restricted Project
MaskRay requested changes to D102986: [lld][ELF][SPARC] Emit RELA entries.

(If the idea is to do the test in a merged patch.)

Wed, Jun 16, 1:38 PM · Restricted Project
MaskRay requested changes to D102287: [MC][X86] Add TrapFillValue definition.

Sorry for being late on reviews.

Wed, Jun 16, 1:36 PM · Restricted Project
MaskRay requested changes to D103974: [llvm-objdump] Add testing for --print-imm-hex, --headers, --section-headers and --private-headers.

Ping:)

Wed, Jun 16, 1:31 PM · Restricted Project
MaskRay accepted D104210: [RISCV][test] Add new tests of SH*ADD in the zba extension.

LGTM.

Wed, Jun 16, 1:31 PM · Restricted Project
MaskRay added a comment to D103815: [ELF] Consider that NOLOAD sections should be placed in a PT_LOAD segment.

After modifying the commit description, use arc diff --verbatim --head=HEAD 'HEAD^' to update the Phabricator subject/summary.

Wed, Jun 16, 1:24 PM · Restricted Project
MaskRay added a comment to D104353: [lld-macho] Avoid force-loading the same archive twice.

Also, have you checked how symlinks/hardlinks behave? link.exe uses sys::fs::getUniqueID.

Wed, Jun 16, 12:36 PM · Restricted Project, Restricted Project
MaskRay added a comment to D104353: [lld-macho] Avoid force-loading the same archive twice.

You may need a test using -force_load for different archives.

Wed, Jun 16, 12:24 PM · Restricted Project, Restricted Project
MaskRay committed rGd619cf5ac5bf: [llvm-objcopy][MachO] Copy LC_LINKER_OPTIMIZATION_HINT (authored by MaskRay).
[llvm-objcopy][MachO] Copy LC_LINKER_OPTIMIZATION_HINT
Wed, Jun 16, 12:10 PM