Page MenuHomePhabricator

Please use GitHub pull requests for new patches. Phabricator shutdown timeline

momo5502 (Maurice Heumann)
User

Projects

User does not belong to any projects.

User Details

User Since
May 19 2020, 6:23 AM (174 w, 4 d)

Recent Activity

Thu, Sep 21

momo5502 abandoned D159099: [CodeGenPrepare] Unmerging GEPs across indirect branches must respect types.

Will be continued as a GitHub PR

Thu, Sep 21, 10:57 PM · Restricted Project, Restricted Project

Tue, Aug 29

momo5502 updated the summary of D159099: [CodeGenPrepare] Unmerging GEPs across indirect branches must respect types.
Tue, Aug 29, 8:33 AM · Restricted Project, Restricted Project
momo5502 added reviewers for D159099: [CodeGenPrepare] Unmerging GEPs across indirect branches must respect types: hjyamauchi, hfinkel.
Tue, Aug 29, 7:53 AM · Restricted Project, Restricted Project
momo5502 updated the summary of D159099: [CodeGenPrepare] Unmerging GEPs across indirect branches must respect types.
Tue, Aug 29, 7:52 AM · Restricted Project, Restricted Project
momo5502 requested review of D159099: [CodeGenPrepare] Unmerging GEPs across indirect branches must respect types.
Tue, Aug 29, 7:51 AM · Restricted Project, Restricted Project

Jul 27 2023

momo5502 abandoned D156396: [AArch64] Respect pre-/post-decrement indexing mode during instruction selection.

I think with fb7c3807, we shouldn't use PRE_DEC/POST_DEC modes anymore. So IsDec will always be false, and this patch does nothing.

Jul 27 2023, 11:26 AM · Restricted Project, Restricted Project
momo5502 edited reviewers for D156396: [AArch64] Respect pre-/post-decrement indexing mode during instruction selection, added: efriedma; removed: eli.friedman.
Jul 27 2023, 12:07 AM · Restricted Project, Restricted Project

Jul 26 2023

momo5502 added a comment to D156396: [AArch64] Respect pre-/post-decrement indexing mode during instruction selection.

Here's a bit more context:

Jul 26 2023, 11:56 PM · Restricted Project, Restricted Project
momo5502 requested review of D156396: [AArch64] Respect pre-/post-decrement indexing mode during instruction selection.
Jul 26 2023, 11:39 PM · Restricted Project, Restricted Project

Jul 12 2023

momo5502 updated the diff for D155093: [DWARF] Fix undefined behaviour in dwarf type printer.

Remove to_string usages

Jul 12 2023, 10:55 PM · Restricted Project, Restricted Project
momo5502 added a comment to D155093: [DWARF] Fix undefined behaviour in dwarf type printer.

Thank you for reviewing, and yes, I totally aggree. Preventing such issues from happening in the first place is would be better.
Will update this change tomorrow to drop the to_string calls.

Jul 12 2023, 12:20 PM · Restricted Project, Restricted Project
momo5502 added a comment to D154575: [X86] Prevent infinite loop in SelectionDAG when lowering negations.

Is there anything missing for this to be committed?

Jul 12 2023, 8:27 AM · Restricted Project, Restricted Project
momo5502 added a reviewer for D155093: [DWARF] Fix undefined behaviour in dwarf type printer: dblaikie.
Jul 12 2023, 8:25 AM · Restricted Project, Restricted Project
momo5502 added a comment to D153800: [ARM] Adjust strd/ldrd codegen alignment requirements.

I created a fix: https://reviews.llvm.org/D155093

Jul 12 2023, 8:24 AM · Restricted Project, Restricted Project
momo5502 requested review of D155093: [DWARF] Fix undefined behaviour in dwarf type printer.
Jul 12 2023, 8:23 AM · Restricted Project, Restricted Project

Jul 11 2023

momo5502 added a comment to D153800: [ARM] Adjust strd/ldrd codegen alignment requirements.

I thought about static_cast'ing Val instead of changing the format string. In case the underlying type of Val changes in the future, one would otherwise have to adapt the format string as well.
Or is there a way to emit a compiler warning if the format string mismatches? You mentioned marking llvm::format with a format attribute, @eli.friedman? Does that trigger a warning?

Jul 11 2023, 11:31 PM · Restricted Project, Restricted Project
momo5502 added a comment to D153800: [ARM] Adjust strd/ldrd codegen alignment requirements.

snprintf(buf, bufsize, "%d", 1LL) has undefined behavior.

Jul 11 2023, 12:20 PM · Restricted Project, Restricted Project
momo5502 added a comment to D153800: [ARM] Adjust strd/ldrd codegen alignment requirements.

It seems that the calling convention snprintf expects is all values in registers, so r0, r1, r2, r3. Meaning it expects the Value to be in r3.

A "long long" can't fit into r3.

The normal calling convention rules (https://github.com/ARM-software/abi-aa/blob/main/aapcs32/aapcs32.rst#parameter-passing) require skipping r3 and storing the "long long" value on the stack. So snprintf should ignore the value in r3 unless we're passing in a bad format string. (Which is possible, I guess... it looks like the "llvm::format" function isn't marked with a format attribute, so the format string isn't checked anywhere.)

Jul 11 2023, 11:48 AM · Restricted Project, Restricted Project
momo5502 added a comment to D153800: [ARM] Adjust strd/ldrd codegen alignment requirements.

I think I figured it out:

Jul 11 2023, 10:28 AM · Restricted Project, Restricted Project

Jul 10 2023

momo5502 added a comment to D153800: [ARM] Adjust strd/ldrd codegen alignment requirements.

Here is the IR code of that specific function. The first load has an align of 4, which used to be excluded by the load optimization before this commit.

Jul 10 2023, 10:41 AM · Restricted Project, Restricted Project

Jul 9 2023

momo5502 added a comment to D153800: [ARM] Adjust strd/ldrd codegen alignment requirements.

I managed to reproduce this and isolate the function responsible for the triggering the bug. It's llvm::format_object<long long>::snprint(char*, unsigned int) const. I haven't dug deeper yet, but that's at least some progress :D

Jul 9 2023, 12:28 PM · Restricted Project, Restricted Project

Jul 6 2023

momo5502 added a comment to D154575: [X86] Prevent infinite loop in SelectionDAG when lowering negations.

cheers - lgtm

Jul 6 2023, 5:13 AM · Restricted Project, Restricted Project
momo5502 added inline comments to D154575: [X86] Prevent infinite loop in SelectionDAG when lowering negations.
Jul 6 2023, 4:17 AM · Restricted Project, Restricted Project
momo5502 added a comment to D153800: [ARM] Adjust strd/ldrd codegen alignment requirements.

@DavidSpickett do you have instructions for me on how to setup everything to reproduce the problem.
So I can support you with the investigations.

Jul 6 2023, 12:50 AM · Restricted Project, Restricted Project
momo5502 requested review of D154575: [X86] Prevent infinite loop in SelectionDAG when lowering negations.
Jul 6 2023, 12:39 AM · Restricted Project, Restricted Project

Jul 3 2023

momo5502 added a comment to D153800: [ARM] Adjust strd/ldrd codegen alignment requirements.

I've reverted this due to a failure on all of Linaro's 2 stage 32 bit Arm bots, both v7 and v8 (though both run on v8 hardware).

For example:
https://lab.llvm.org/buildbot/#/builders/178/builds/5128
https://lab.llvm.org/buildbot/#/builders/187/builds/11994
https://lab.llvm.org/buildbot/#/builders/182/builds/6724

            7: error: Simplified template DW_AT_name could not be reconstituted: 
check:10'0     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
            8:  original: f3<unsigned char, (unsigned char)'\x00'> 
check:10'0     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
            9:  reconstituted: f3<unsigned char, (unsigned char)'\x7f'>

I haven't dug deeper yet but if that is one byte misplaced, it could make some sense.

Do you have access to a machine where you can reproduce this? A 32 bit container running on 64 bit hardware will also work.

If you do not, I can do the investigation.

Jul 3 2023, 7:24 AM · Restricted Project, Restricted Project

Jun 26 2023

momo5502 added a comment to D153800: [ARM] Adjust strd/ldrd codegen alignment requirements.

LGTM

I suspect we're actually missing a check somewhere for LDRD formation post-RA. But this looks like an improvement.

(I'll commit for you.)

Jun 26 2023, 12:36 PM · Restricted Project, Restricted Project
momo5502 requested review of D153800: [ARM] Adjust strd/ldrd codegen alignment requirements.
Jun 26 2023, 11:59 AM · Restricted Project, Restricted Project

Jun 23 2023

momo5502 updated subscribers of D152790: [ARM] Fix codegen of unaligned volatile load/store of i64.

as soon as this is commited,I will try to adjust the ARMLoadStoreOptimizer accordingly. @eli.friedman you have commit permissions, right? would you mind committing this change for me with the name and email provided above?

Jun 23 2023, 11:59 PM · Restricted Project, Restricted Project

Jun 21 2023

momo5502 added a comment to D152790: [ARM] Fix codegen of unaligned volatile load/store of i64.

LGTM

(I'm guessing you don't have commit access? Please let me know the author name/email you want on the commit.)

Jun 21 2023, 11:13 AM · Restricted Project, Restricted Project
momo5502 updated the diff for D152790: [ARM] Fix codegen of unaligned volatile load/store of i64.

Thank you for the hint. Using update_llc_test_checks.py makes writing the tests much easier.

Jun 21 2023, 1:45 AM · Restricted Project, Restricted Project

Jun 20 2023

momo5502 updated the diff for D152790: [ARM] Fix codegen of unaligned volatile load/store of i64.

Added an armv7 strict case to make sure it is identical to non-strict for align 4

Jun 20 2023, 12:17 AM · Restricted Project, Restricted Project
momo5502 updated the diff for D152790: [ARM] Fix codegen of unaligned volatile load/store of i64.

I added an align 4 test for armv7 and armv6 with and without strict alignment.
I omitted armv5 here, as, due to the missing strict align, the test generates an ldrd which is probably not something one wants to explicitly test for.
Question is, should I add strict align to armv4/v5 test cases here and adjust the test accordingly?

Jun 20 2023, 12:11 AM · Restricted Project, Restricted Project

Jun 19 2023

momo5502 updated the diff for D152790: [ARM] Fix codegen of unaligned volatile load/store of i64.

Thank you for investigating it. I have added a helper called getDualLoadStoreAlignment. Feel free to suggest a better name if you have one.

Jun 19 2023, 2:00 AM · Restricted Project, Restricted Project

Jun 15 2023

momo5502 updated the diff for D152790: [ARM] Fix codegen of unaligned volatile load/store of i64.

I have no idea why the ARMLoadStoreOptimizer code was written that way; maybe fix that too, for the sake of clarity.

Jun 15 2023, 3:04 AM · Restricted Project, Restricted Project

Jun 13 2023

momo5502 added a comment to D152790: [ARM] Fix codegen of unaligned volatile load/store of i64.

I did have Align(4) initially, but saw that it was done differently here: https://github.com/llvm/llvm-project/blob/2a1716dec57e8b3dd668df17ecbedfc77a4112e5/llvm/lib/Target/ARM/ARMLoadStoreOptimizer.cpp#L2294
I simply copied the expression. If that is not applicable here, I can just use 4 instead or respect allowsUnalignedMem.

Jun 13 2023, 9:58 PM · Restricted Project, Restricted Project
momo5502 requested review of D152790: [ARM] Fix codegen of unaligned volatile load/store of i64.
Jun 13 2023, 2:08 AM · Restricted Project, Restricted Project

Feb 6 2022

momo5502 added a comment to D117500: [clang] Mention MS on-demand TLS initialization in release notes.

Seems like this can be merged, right?

Feb 6 2022, 2:56 AM · Restricted Project

Jan 17 2022

momo5502 added a comment to D115456: [MS] Implement on-demand TLS initialization for Microsoft CXX ABI.

Looks great, thanks! Maurice, do you want to add a note about this to docs/ReleaseNotes.rst?

Jan 17 2022, 9:22 AM · Restricted Project, Restricted Project
momo5502 requested review of D117500: [clang] Mention MS on-demand TLS initialization in release notes.
Jan 17 2022, 9:21 AM · Restricted Project

Jan 3 2022

momo5502 updated the diff for D115456: [MS] Implement on-demand TLS initialization for Microsoft CXX ABI.

This is looking great! Just a few more questions.

What is the behavior with something like:

thread_local int x = 2;
int f() {
  return x;
}

I'm wondering if we need to move this logic into the generic C++ ABI implementation.

The MS compiler only emits the dynamic initializers for variables with constructors/destructors, just like it is currently done here for the Itanium ABI.
I also thought about adopting that behaviour, but I think threre are edge-cases when triggering dynamic TLS initialization even for constant variables is useful.
For example there might be custom TLS callbacks that can affect the value of this variable.

If desired, I can change it to match the behaviour of MS, but I thought it could be beneficial to diverge in this case.

IMO, it is probably best to match behavior here within reason (ignoring bugs latent in MSVC) here. My thinking is that in the face of COMDATs, we cannot ensure which copy of an inline function will make its way to the binary and different link orders would provide different behavior.

Jan 3 2022, 11:04 AM · Restricted Project, Restricted Project
momo5502 added a comment to D115456: [MS] Implement on-demand TLS initialization for Microsoft CXX ABI.

This is looking great! Just a few more questions.

What is the behavior with something like:

thread_local int x = 2;
int f() {
  return x;
}

I'm wondering if we need to move this logic into the generic C++ ABI implementation.

Jan 3 2022, 8:31 AM · Restricted Project, Restricted Project

Dec 31 2021

momo5502 updated the diff for D115456: [MS] Implement on-demand TLS initialization for Microsoft CXX ABI.

Comments were addressed

Dec 31 2021, 6:12 AM · Restricted Project, Restricted Project

Dec 22 2021

momo5502 updated the diff for D115456: [MS] Implement on-demand TLS initialization for Microsoft CXX ABI.

A call to isCompatibleWithMSVC was added with the proper version that introduced the change (1925) and comments describing the intention of the change were added.

Dec 22 2021, 6:11 AM · Restricted Project, Restricted Project

Dec 9 2021

momo5502 updated the diff for D115456: [MS] Implement on-demand TLS initialization for Microsoft CXX ABI.
Dec 9 2021, 12:00 PM · Restricted Project, Restricted Project
momo5502 added a reviewer for D115456: [MS] Implement on-demand TLS initialization for Microsoft CXX ABI: rsmith.
Dec 9 2021, 10:54 AM · Restricted Project, Restricted Project
momo5502 updated the summary of D115456: [MS] Implement on-demand TLS initialization for Microsoft CXX ABI.
Dec 9 2021, 10:35 AM · Restricted Project, Restricted Project
momo5502 requested review of D115456: [MS] Implement on-demand TLS initialization for Microsoft CXX ABI.
Dec 9 2021, 10:29 AM · Restricted Project, Restricted Project