Page MenuHomePhabricator

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

hvdijk (Harald van Dijk)
User

Projects

User does not belong to any projects.

User Details

User Since
Aug 13 2019, 4:58 AM (216 w, 1 d)

Recent Activity

Thu, Sep 21

hvdijk added a comment to D86310: [X86] Align i128 to 16 bytes in x86 datalayouts.

I do not think there is a sensible way to keep upgrade-datalayout2.ll working, with the way the upgrade logic is structured, and we should rethink that test. The change here intends to insert -i128:128- into x86 data layouts that do not have it. The goal of upgrade-datalayout2.ll is to test that data layouts that are not valid x86 data layouts do not get upgraded. However, I see no sensible logic by which we can say that in this particular case, we should not add it.

Thu, Sep 21, 5:38 PM · Restricted Project, Restricted Project, Restricted Project
hvdijk updated the diff for D124406: [X86] Use indirect addressing for high 2GB of x32 address space.

Rebased.

Thu, Sep 21, 3:27 AM · Restricted Project, Restricted Project

Wed, Sep 20

hvdijk added inline comments to D86310: [X86] Align i128 to 16 bytes in x86 datalayouts.
Wed, Sep 20, 10:03 AM · Restricted Project, Restricted Project, Restricted Project
hvdijk added a comment to D86310: [X86] Align i128 to 16 bytes in x86 datalayouts.

I'm happy to sign off on the x86-64 part here, but I'm less sure about x86-32. If I understood correctly, the i128 alignment is raised there exclusively to fix the "f128 legalized to i128 libcall" case -- is there any other ABI requirement for i128 alignment on x86-32? Is raising i128 alignment the right way to fix an f128 issue?

Wed, Sep 20, 12:44 AM · Restricted Project, Restricted Project, Restricted Project

Tue, Sep 19

hvdijk added a comment to D86310: [X86] Align i128 to 16 bytes in x86 datalayouts.

Is this just waiting for a review?

Tue, Sep 19, 2:10 PM · Restricted Project, Restricted Project, Restricted Project

Mon, Sep 18

hvdijk added a comment to D124406: [X86] Use indirect addressing for high 2GB of x32 address space.

@hvdijk reverse-ping - what happended with this patch?

Mon, Sep 18, 1:29 AM · Restricted Project, Restricted Project

Mon, Sep 11

hvdijk added a comment to D86310: [X86] Align i128 to 16 bytes in x86 datalayouts.

I think that D158169 seems to have fixed clang as well; after applying both patches, clang gcc and rustc all seem to agree.

Interesting. I cannot see how it would, I may be missing something; I will check when I am able.

D158169 landed today, I confirmed that the current main (with D158169) makes Clang <-> GCC works but LLVM still fails without this patch.

Mon, Sep 11, 6:32 PM · Restricted Project, Restricted Project, Restricted Project

Aug 17 2023

hvdijk added a comment to D86310: [X86] Align i128 to 16 bytes in x86 datalayouts.

I think that D158169 seems to have fixed clang as well; after applying both patches, clang gcc and rustc all seem to agree.

Aug 17 2023, 5:12 PM · Restricted Project, Restricted Project, Restricted Project
hvdijk added a comment to D86310: [X86] Align i128 to 16 bytes in x86 datalayouts.

Is clang still doing something wrong? From my testing, it seems like clang and GCC now agree with each other, I am not sure what would still be incorrect

Aug 17 2023, 2:18 PM · Restricted Project, Restricted Project, Restricted Project
hvdijk added a comment to D86310: [X86] Align i128 to 16 bytes in x86 datalayouts.

@nikic posted a patch that fixes the register passing at https://reviews.llvm.org/D158169. I think that patch plus this one should resolve all the problems we have

Aug 17 2023, 12:30 PM · Restricted Project, Restricted Project, Restricted Project

Jul 20 2023

hvdijk added a comment to D86310: [X86] Align i128 to 16 bytes in x86 datalayouts.

Does this happen on the clang side or the LLVM side?

Jul 20 2023, 10:52 AM · Restricted Project, Restricted Project, Restricted Project

Jul 19 2023

hvdijk added a comment to D86310: [X86] Align i128 to 16 bytes in x86 datalayouts.

Just FYI. There are a few reports about the compatibility issues, e.g., #41784.

Jul 19 2023, 6:45 PM · Restricted Project, Restricted Project, Restricted Project
hvdijk added a comment to D86310: [X86] Align i128 to 16 bytes in x86 datalayouts.

Is the compatibility note here only meant to address calls between LLVM and GCC, not generated code? Because of course, struct layout and pass-in-memory function calls are incompatible.

Jul 19 2023, 1:56 PM · Restricted Project, Restricted Project, Restricted Project

Jul 16 2023

hvdijk added a comment to D86310: [X86] Align i128 to 16 bytes in x86 datalayouts.

THE CURRENT STATE

Jul 16 2023, 6:22 AM · Restricted Project, Restricted Project, Restricted Project
hvdijk updated the diff for D86310: [X86] Align i128 to 16 bytes in x86 datalayouts.

Rebased on current LLVM. No longer limited to 64 bit mode. 32 bit mode was taken out in D28990 because it was not believed to be required for compatibility, but as PR50198 shows, it does cause a compatibility issue to leave that as is.

Jul 16 2023, 5:44 AM · Restricted Project, Restricted Project, Restricted Project
hvdijk commandeered D86310: [X86] Align i128 to 16 bytes in x86 datalayouts.
Jul 16 2023, 5:38 AM · Restricted Project, Restricted Project, Restricted Project

Jul 14 2023

hvdijk added a comment to D86310: [X86] Align i128 to 16 bytes in x86 datalayouts.
In D86310#4501240, @rnk wrote:

For example, it would generally be better if long double were 8-byte-aligned, but the x86 32-bit ABI specifies that it is 4-byte-aligned, and that is set in stone. I would be against any change in LLVM's ABI that changed their alignment, even if it would speed up code.

That may be your view, but other users rely on the -malign-double flag (D19734) to get the new behavior, despite the ABI concerns. Specifically, it mattered for users passing structs from CPU to GPU, because the GPU doesn't tolerate misaligned doubles well. With that in mind, I wouldn't describe this ABI rule as being "set in stone", but I understand your perspective.

Jul 14 2023, 9:04 AM · Restricted Project, Restricted Project, Restricted Project
hvdijk added a comment to D86310: [X86] Align i128 to 16 bytes in x86 datalayouts.

@JohnReagan That is a valid concern, and I hope it reassures you that if things were working before, I would never be on board with this change. For example, it would generally be better if long double were 8-byte-aligned, but the x86 32-bit ABI specifies that it is 4-byte-aligned, and that is set in stone. I would be against any change in LLVM's ABI that changed their alignment, even if it would speed up code. I still occasionally run 20-year-old binaries, myself, that are dynamically linked to shared object files built with current compilers. Compatibility matters, I would not be on board with a change that breaks things like that. But that is not what is happening here. For i128, what clang implemented matched GCC, what LLVM implemented deviated from GCC/clang, but LLVM assumed that its implementation actually did match GCC/clang and code crashed as a result. This change would make it so that LLVM starts to also match GCC/clang, to change things from something that doesn't work to something that does work, and because things crash in current LLVM, I do not believe there can be much code out there that relies on the current behaviour. As you say, you aren't using i128/f128 yourself either. I hope that when I can update the patch, you can check that this does not cause problems for you.

Jul 14 2023, 8:47 AM · Restricted Project, Restricted Project, Restricted Project

Jul 13 2023

hvdijk added a comment to D86310: [X86] Align i128 to 16 bytes in x86 datalayouts.

https://reviews.llvm.org/D86310#2231136 has an example where IR generated by clang breaks.

Jul 13 2023, 12:23 PM · Restricted Project, Restricted Project, Restricted Project
hvdijk added a comment to D86310: [X86] Align i128 to 16 bytes in x86 datalayouts.
In D86310#4498551, @rnk wrote:

Given that most non-clang frontends want the bug fix (ABI break), who exactly is asking for this level of IR ABI stability?

Jul 13 2023, 11:30 AM · Restricted Project, Restricted Project, Restricted Project
hvdijk added a comment to D86310: [X86] Align i128 to 16 bytes in x86 datalayouts.

The main problem with that is that we can't have multiple data layouts for one module, so linking old and new bitcode together would fail.

Jul 13 2023, 9:45 AM · Restricted Project, Restricted Project, Restricted Project

Jul 12 2023

hvdijk added a comment to D86310: [X86] Align i128 to 16 bytes in x86 datalayouts.

A thought occurs: in older versions of LLVM, the data layout mechanism worked differently and permitted targets to declare that they supported multiple different data layout strings, by overriding isCompatibleDataLayout. This mechanism was removed in D67631. If we reinstate that, we can have the X86 target declare that it "supports" data layout strings with and without the -i128:128, where by "supports", I mean the code continues to not generally work in the same way it does not generally work now, but the specific limited cases that do work continue to work exactly the same ABI-incompatible way. This would have the same result of bug-for-bug compatibility with existing modules, but in what I suspect would be a significantly simpler way than by going through the module and adding explicit alignments everywhere. While I would still prefer to give up on that compatibility, if it is a hard requirement, and if this would be an alternative way of achieving it, I might possibly be able to update this patch to do just that. Would this be acceptable?

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

Jul 10 2023

hvdijk added a comment to D86310: [X86] Align i128 to 16 bytes in x86 datalayouts.

What is the current status of this patch? Are the reviewers here OK with this fix in general but just need to see changes to autoupgrade?

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

Apr 17 2023

hvdijk added a comment to rG5ad2aae24474: [ELF] SharedFile::parse: move verdefIndex assignment outside of ctor. NFC.

https://github.com/llvm/llvm-project/issues/62194

Apr 17 2023, 9:47 PM
hvdijk added a comment to rG5ad2aae24474: [ELF] SharedFile::parse: move verdefIndex assignment outside of ctor. NFC.

In my https://sourceware.org/pipermail/binutils/2022-November/124312.html example, fuse should simply remove FUSE_SYMVER(".symver fuse_new_compat2,fuse_new@");, which will remove the problematic unversioned fuse_new symbol.

Apr 17 2023, 9:28 PM
hvdijk added a comment to rG5ad2aae24474: [ELF] SharedFile::parse: move verdefIndex assignment outside of ctor. NFC.
Apr 17 2023, 7:34 PM

Apr 12 2023

hvdijk added a comment to D140315: [AMDGCN] Update search path for device libraries.

I'm sorry for reporting it this late (clang's been broken for over two weeks, so we couldn't periodically test it) but this change is causing test regressions on Gentoo/amd64:

Thanks for reporting the issue. I'm working on a patch to fix the test.

Apr 12 2023, 4:42 PM · Restricted Project, Restricted Project

Apr 4 2023

hvdijk added a comment to D110900: Triple: Add RedHat vendor.
Apr 4 2023, 1:53 AM · Restricted Project, Restricted Project

Apr 3 2023

hvdijk added a comment to D110900: Triple: Add RedHat vendor.

Can you elaborate? I agree that decreasing the number of filesystem stats is preferable...

Apr 3 2023, 8:58 PM · Restricted Project, Restricted Project

Apr 2 2023

hvdijk added a comment to D110900: Triple: Add RedHat vendor.

In D110900#3044136, I suggested another approach that should reduce the number of file system accesses, which you implemented in D110900#3044545 (D111207). Apologies for never responding to that; at first glance the way you implemented that looks like it increases the number of file system accesses, and I don't think we should do that. Given that the only implemented alternative is this one, I'm not going to argue against this.

Apr 2 2023, 2:29 PM · Restricted Project, Restricted Project

Mar 29 2023

hvdijk committed rG6b868139458d: [SYCL] Always set NoUnwind attribute for SYCL. (authored by hvdijk).
[SYCL] Always set NoUnwind attribute for SYCL.
Mar 29 2023, 6:19 PM · Restricted Project, Restricted Project
hvdijk closed D147097: [SYCL] Always set NoUnwind attribute for SYCL..
Mar 29 2023, 6:19 PM · Restricted Project, Restricted Project
hvdijk updated the diff for D147097: [SYCL] Always set NoUnwind attribute for SYCL..

Forgot to update the call to foo() to call bar() instead.

Mar 29 2023, 6:57 AM · Restricted Project, Restricted Project
hvdijk updated the diff for D147097: [SYCL] Always set NoUnwind attribute for SYCL..

Apparently the test was already passing without my change, so I updated the test to make it a function that did not previously get the nounwind attribute. This also revealed that the test was more fragile than I realised: it depends on the precise order in which functions are emitted. We always match "Function Attrs:" to the first emitted function, and then CHECK-NEXT can fail. We'd ideally first CHECK for void @_Z3foov() (now void @_Z3barv()) and then CHECK-PREV for the attributes, but FileCheck does not support that. What we can do instead though is find the matching attributes #n = { ... } line and verify that everything is in there; updated accordingly.

Mar 29 2023, 6:29 AM · Restricted Project, Restricted Project

Mar 28 2023

hvdijk added a comment to D147097: [SYCL] Always set NoUnwind attribute for SYCL..

That's a good question. I haven't looked into this issue deep enough, but I think using -fexceptions requires using delayed diagnostics to avoid false diagnostics during host code analysis.

Mar 28 2023, 6:12 PM · Restricted Project, Restricted Project
hvdijk added a comment to D147097: [SYCL] Always set NoUnwind attribute for SYCL..

Is the rationale I gave in the description correct, or would it be better for SYCL device code to unconditionally build without -fexceptions and get the nounwind attribute added that way?

Mar 28 2023, 5:40 PM · Restricted Project, Restricted Project
hvdijk requested review of D147097: [SYCL] Always set NoUnwind attribute for SYCL..
Mar 28 2023, 5:39 PM · Restricted Project, Restricted Project

Mar 9 2023

hvdijk added inline comments to D106129: [ExecutionEngine] Check for libunwind before calling __register_frame.
Mar 9 2023, 2:45 PM · Restricted Project, Restricted Project

Nov 10 2022

hvdijk added a comment to rG5ad2aae24474: [ELF] SharedFile::parse: move verdefIndex assignment outside of ctor. NFC.

Your last comment reads like things only break if FUSE 2 is linked with ld.bfd, and ntfs3g with ld.lld, that ld.bfd is able to produce something that ld.lld is unable to process as intended. For the avoidance of doubt, on my system, FUSE 2 was built with clang+lld, so any scenario that lld cannot produce is not something that could have come up.

No, things only break if libfuse 2 is linked with ld.lld (which cannot produce VER_NDX_GLOBAL VERSYM_HIDDEN) and has fuse_new@@FUSE_2.6 before fuse_new (VER_NDX_GLOBAL) and a program links against libfuse.so.

Nov 10 2022, 9:32 PM
hvdijk added a comment to rG5ad2aae24474: [ELF] SharedFile::parse: move verdefIndex assignment outside of ctor. NFC.

Your last comment reads like things only break if FUSE 2 is linked with ld.bfd, and ntfs3g with ld.lld, that ld.bfd is able to produce something that ld.lld is unable to process as intended. For the avoidance of doubt, on my system, FUSE 2 was built with clang+lld, so any scenario that lld cannot produce is not something that could have come up.

Nov 10 2022, 9:06 PM
hvdijk added a comment to rG5ad2aae24474: [ELF] SharedFile::parse: move verdefIndex assignment outside of ctor. NFC.

Note that in the version on my system, fuse_new appears after fuse_new@@FUSE_2.6, whereas in the version on your system, it appears before. I have not checked in detail what is happening in this commit. Is it possible that for whatever reason it is causing whichever version is seen last to take priority? If so, it is unfortunate that you will not be able to reproduce the problem on your system with the Debian-provided version of FUSE. I should be able to come up with a standalone test that does not rely on FUSE, but it will take longer.

It's difficult to tell without a standalone reproducer...

Nov 10 2022, 4:07 PM

Nov 9 2022

hvdijk added a comment to rG5ad2aae24474: [ELF] SharedFile::parse: move verdefIndex assignment outside of ctor. NFC.

The symbols in the DSO are:

% readelf -W --dyn-syms /lib/x86_64-linux-gnu/libfuse.so | grep fuse_new

200: 00000000000148b0    72 FUNC    GLOBAL DEFAULT   13 fuse_new@FUSE_2.5
201: 0000000000014870    16 FUNC    GLOBAL DEFAULT   13 fuse_new
202: 0000000000014750     8 FUNC    GLOBAL DEFAULT   13 fuse_new@@FUSE_2.6
203: 0000000000014860    11 FUNC    GLOBAL DEFAULT   13 fuse_new@FUSE_2.2
216: 0000000000014880    35 FUNC    GLOBAL DEFAULT   13 fuse_new_compat1@@FUSE_2.2
218: 0000000000014870    16 FUNC    GLOBAL DEFAULT   13 fuse_new_compat2@@FUSE_2.2
228: 0000000000014860    11 FUNC    GLOBAL DEFAULT   13 fuse_new_compat22@@FUSE_2.5
230: 00000000000148b0    72 FUNC    GLOBAL DEFAULT   13 fuse_new_compat25@@FUSE_2.6
Nov 9 2022, 6:37 PM
hvdijk added a comment to rG5ad2aae24474: [ELF] SharedFile::parse: move verdefIndex assignment outside of ctor. NFC.

This was committed as NFC, but isn't NFC and did end up breaking code.

Nov 9 2022, 4:44 PM

Jun 28 2022

hvdijk added inline comments to D127119: [SLP]Fix undef handling in gather function..
Jun 28 2022, 12:03 PM · Restricted Project, Restricted Project

Jun 12 2022

hvdijk added a comment to D127119: [SLP]Fix undef handling in gather function..

Thanks, I am seeing improvements with this new version. I'll try to go over the changes in more detail later, some initial superficial comments now.

Jun 12 2022, 9:58 AM · Restricted Project, Restricted Project

Jun 7 2022

hvdijk added a comment to D127119: [SLP]Fix undef handling in gather function..

Comparing the changes to the tests in this diff to those in D127073, I am seeing a number of tests where we have more shufflevectors, and none where we have fewer. Are there improvements that are not as obvious to see?

Yes, there are. These extra shuffles caused by changes in performExtractsShuffleAction() and in IsIdenticalOrLessDefined lambda, these changes (they treat UndefMaskElem as possible poison) increase number of shuffles. Without them, there are less shuffles, these extra changes are required for correct handling of UndefMaskElem as posion.

Jun 7 2022, 12:45 PM · Restricted Project, Restricted Project
hvdijk added a comment to D127119: [SLP]Fix undef handling in gather function..

Comparing the changes to the tests in this diff to those in D127073, I am seeing a number of tests where we have more shufflevectors, and none where we have fewer. Are there improvements that are not as obvious to see?

Jun 7 2022, 12:11 PM · Restricted Project, Restricted Project

Jun 6 2022

hvdijk added inline comments to D127119: [SLP]Fix undef handling in gather function..
Jun 6 2022, 1:27 PM · Restricted Project, Restricted Project
hvdijk added inline comments to D127119: [SLP]Fix undef handling in gather function..
Jun 6 2022, 12:19 PM · Restricted Project, Restricted Project
hvdijk added inline comments to D127119: [SLP]Fix undef handling in gather function..
Jun 6 2022, 12:13 PM · Restricted Project, Restricted Project
hvdijk added inline comments to D127119: [SLP]Fix undef handling in gather function..
Jun 6 2022, 11:51 AM · Restricted Project, Restricted Project
hvdijk added inline comments to D127119: [SLP]Fix undef handling in gather function..
Jun 6 2022, 11:33 AM · Restricted Project, Restricted Project

Jun 5 2022

hvdijk added inline comments to D127073: [SLP] Treat undef as any other constant.
Jun 5 2022, 12:13 PM · Restricted Project, Restricted Project
hvdijk added inline comments to D127073: [SLP] Treat undef as any other constant.
Jun 5 2022, 9:53 AM · Restricted Project, Restricted Project
hvdijk added inline comments to D127073: [SLP] Treat undef as any other constant.
Jun 5 2022, 9:24 AM · Restricted Project, Restricted Project
hvdijk added inline comments to D127073: [SLP] Treat undef as any other constant.
Jun 5 2022, 8:59 AM · Restricted Project, Restricted Project
hvdijk added inline comments to D127073: [SLP] Treat undef as any other constant.
Jun 5 2022, 8:40 AM · Restricted Project, Restricted Project
hvdijk added inline comments to D127073: [SLP] Treat undef as any other constant.
Jun 5 2022, 8:18 AM · Restricted Project, Restricted Project
hvdijk added inline comments to D127073: [SLP] Treat undef as any other constant.
Jun 5 2022, 8:13 AM · Restricted Project, Restricted Project
hvdijk added inline comments to D127073: [SLP] Treat undef as any other constant.
Jun 5 2022, 8:05 AM · Restricted Project, Restricted Project
hvdijk added inline comments to D126939: [SLP] Avoid converting undef to poison when gathering..
Jun 5 2022, 7:52 AM · Restricted Project, Restricted Project
hvdijk requested review of D127073: [SLP] Treat undef as any other constant.
Jun 5 2022, 7:51 AM · Restricted Project, Restricted Project
hvdijk added inline comments to D126939: [SLP] Avoid converting undef to poison when gathering..
Jun 5 2022, 5:09 AM · Restricted Project, Restricted Project
hvdijk added inline comments to D126939: [SLP] Avoid converting undef to poison when gathering..
Jun 5 2022, 4:13 AM · Restricted Project, Restricted Project
hvdijk added inline comments to D126939: [SLP] Avoid converting undef to poison when gathering..
Jun 5 2022, 2:54 AM · Restricted Project, Restricted Project

Jun 4 2022

hvdijk added inline comments to D126939: [SLP] Avoid converting undef to poison when gathering..
Jun 4 2022, 7:26 PM · Restricted Project, Restricted Project

Jun 3 2022

hvdijk added a comment to D126939: [SLP] Avoid converting undef to poison when gathering..

The updates to the tests suggest that this doesn't just fix the broken cases, it generates worse code where the previously generated code was already correct: I'm seeing insertelements into a vector immediately followed by a shufflevector that discards the inserted element, where previously those elements were not set. Is there some way that we can avoid that?

Jun 3 2022, 9:44 PM · Restricted Project, Restricted Project

Jun 2 2022

hvdijk added a comment to D126895: [SLP] Phi inputs that come from an unreachable block should be undef..

I think that the issue boils down to whether: %zext = zext i8 poison to i32 is an i32 poison.

Jun 2 2022, 11:50 AM · Restricted Project, Restricted Project
hvdijk added a comment to D126895: [SLP] Phi inputs that come from an unreachable block should be undef..

The value coming from an unreachable block from entry is uninitialized

Jun 2 2022, 10:41 AM · Restricted Project, Restricted Project

May 3 2022

hvdijk added inline comments to D122663: Mark identifier prefixes as substitutable.
May 3 2022, 5:08 AM · Restricted Project, Restricted Project

May 2 2022

hvdijk committed rGfed7be096f8e: Mark identifier prefixes as substitutable (authored by hvdijk).
Mark identifier prefixes as substitutable
May 2 2022, 10:08 AM · Restricted Project, Restricted Project
hvdijk closed D122663: Mark identifier prefixes as substitutable.
May 2 2022, 10:08 AM · Restricted Project, Restricted Project
hvdijk added a comment to D122663: Mark identifier prefixes as substitutable.

Ping me EOW if @rsmith doesn't respond in the meantime.

May 2 2022, 8:36 AM · Restricted Project, Restricted Project

Apr 26 2022

hvdijk added inline comments to D124406: [X86] Use indirect addressing for high 2GB of x32 address space.
Apr 26 2022, 11:57 PM · Restricted Project, Restricted Project
hvdijk added inline comments to D124406: [X86] Use indirect addressing for high 2GB of x32 address space.
Apr 26 2022, 5:19 PM · Restricted Project, Restricted Project
hvdijk updated the diff for D124406: [X86] Use indirect addressing for high 2GB of x32 address space.

Add a comment explaining why we do not use address size overrides here.

Apr 26 2022, 5:10 PM · Restricted Project, Restricted Project

Apr 25 2022

hvdijk added inline comments to D124406: [X86] Use indirect addressing for high 2GB of x32 address space.
Apr 25 2022, 3:26 PM · Restricted Project, Restricted Project
hvdijk added inline comments to D124406: [X86] Use indirect addressing for high 2GB of x32 address space.
Apr 25 2022, 12:58 PM · Restricted Project, Restricted Project
hvdijk added inline comments to D124406: [X86] Use indirect addressing for high 2GB of x32 address space.
Apr 25 2022, 12:24 PM · Restricted Project, Restricted Project
hvdijk updated the diff for D124406: [X86] Use indirect addressing for high 2GB of x32 address space.

Clean up test slightly to load consecutive i32 values like the i64 version did

Apr 25 2022, 10:40 AM · Restricted Project, Restricted Project
hvdijk requested review of D124406: [X86] Use indirect addressing for high 2GB of x32 address space.
Apr 25 2022, 10:14 AM · Restricted Project, Restricted Project
hvdijk added a comment to D122663: Mark identifier prefixes as substitutable.

This actually wouldn't be necessary in this case: cxxfilt is already 'right', this is just for humans to be able to see "we changed this from mangling as <obviously wrong> to <looks right>".

Apr 25 2022, 8:13 AM · Restricted Project, Restricted Project
hvdijk added a comment to D122663: Mark identifier prefixes as substitutable.

I DID see that and am REASONABLY sure you do, but sometimes folks will ask for test coverage because they suspect the resulting behavior will demonstrate some sort of problem with the current patch. I DID run this through the demangler and found that the PRE15 case looks odd?

void test10<X>(X::Y::a, X::Y::b, float*, X::Y)
vs
void test10<X>(X::Y::a, X::Y::b, float*, float*)

Or is that exactly the bug being caught here? That is, is the float* substitution not being properly registered/emitted and being confused with X::Y?

Apr 25 2022, 7:19 AM · Restricted Project, Restricted Project
hvdijk added a comment to D122663: Mark identifier prefixes as substitutable.

Ping me EOW if @rsmith doesn't respond in the meantime. It is also not clear to me whether you were able to capture/fix the issue he had with the clang-abi-compat.cpp test.

Apr 25 2022, 7:02 AM · Restricted Project, Restricted Project

Apr 24 2022

hvdijk added a comment to D122663: Mark identifier prefixes as substitutable.

LGTM! I would like @rjmccall to take a pass if he ends up having time in the next day or two (perhaps tack on an extra day or two because of Easter), else I'll be willing to approve later in the week.

Apr 24 2022, 3:39 PM · Restricted Project, Restricted Project

Apr 20 2022

hvdijk updated the diff for D122663: Mark identifier prefixes as substitutable.

Fixed release notes to use correct RST syntax.

Apr 20 2022, 5:34 AM · Restricted Project, Restricted Project

Apr 19 2022

hvdijk added inline comments to D122663: Mark identifier prefixes as substitutable.
Apr 19 2022, 4:43 PM · Restricted Project, Restricted Project

Apr 18 2022

hvdijk added inline comments to D122663: Mark identifier prefixes as substitutable.
Apr 18 2022, 4:17 PM · Restricted Project, Restricted Project
hvdijk updated the diff for D122663: Mark identifier prefixes as substitutable.

Extend test, add assert.

Apr 18 2022, 4:16 PM · Restricted Project, Restricted Project
hvdijk updated the diff for D122663: Mark identifier prefixes as substitutable.

Add to release notes.

Apr 18 2022, 12:07 PM · Restricted Project, Restricted Project
hvdijk added a comment to D122663: Mark identifier prefixes as substitutable.

Our most recent direction is to document any non-NFC patches in the release notes if at all sensible, so I think this meets those requirements.

Apr 18 2022, 11:52 AM · Restricted Project, Restricted Project
hvdijk added a comment to D122663: Mark identifier prefixes as substitutable.

I believe the answer here is 'yes'. We also likely need release notes.

Apr 18 2022, 10:57 AM · Restricted Project, Restricted Project
hvdijk updated the diff for D122663: Mark identifier prefixes as substitutable.

Allow the old mangling with suitable -fclang-abi-compat= value

Apr 18 2022, 10:55 AM · Restricted Project, Restricted Project
hvdijk updated subscribers of D122663: Mark identifier prefixes as substitutable.

ping. Apologies, I don't know who to add as a reviewer, there is no one specifically listed as code owner and it does not seem to be handled by anyone in particular. @urnathan, @erichkeane, you two appear to be the most recent people who pushed commits specifically for mangling issues, would either of you be able to review this?

Apr 18 2022, 6:39 AM · Restricted Project, Restricted Project

Apr 12 2022

hvdijk accepted D122449: [X86][test] Add encoding/decoding tests for VEX instruction w/ address-size prefix.

I've pushed D122540, would you like me to push this as well or would you rather do that yourself?

Apr 12 2022, 10:34 AM · Restricted Project, Restricted Project
hvdijk committed rG3337f50625a3: [X86] Fix handling of maskmovdqu in x32 differently (authored by hvdijk).
[X86] Fix handling of maskmovdqu in x32 differently
Apr 12 2022, 10:33 AM · Restricted Project, Restricted Project
hvdijk closed D122540: [X86] Fix handling of maskmovdqu in x32 differently.
Apr 12 2022, 10:32 AM · Restricted Project, Restricted Project

Apr 11 2022

hvdijk added a comment to D122540: [X86] Fix handling of maskmovdqu in x32 differently.

I mentioned that I'd prefer to get another person to review this as well but in the interest of just getting the bug fixed, I'll merge this tomorrow if there is no more feedback.

Apr 11 2022, 4:37 PM · Restricted Project, Restricted Project

Apr 6 2022

hvdijk added inline comments to D122540: [X86] Fix handling of maskmovdqu in x32 differently.
Apr 6 2022, 1:33 AM · Restricted Project, Restricted Project
hvdijk updated the diff for D122540: [X86] Fix handling of maskmovdqu in x32 differently.
Apr 6 2022, 1:31 AM · Restricted Project, Restricted Project