Page MenuHomePhabricator

amharc (Krzysztof Pszeniczny)
User

Projects

User does not belong to any projects.

User Details

User Since
Jan 20 2017, 3:09 PM (185 w, 1 h)

Recent Activity

Sat, Jul 11

amharc added a comment to D79978: Call Frame Information (CFI) Handling for Basic Block Sections.

I cannot find a test with .cfa_offset or describing a non-rbp register, so the implementation is under-tested. How about leveraging inline asm to clobber callee-saved registers?

Sat, Jul 11, 1:22 PM · Restricted Project

Fri, Jul 10

amharc added a comment to D79978: Call Frame Information (CFI) Handling for Basic Block Sections.
In D79978#2141992, @wmi wrote:

Ack. Then what instructions should be placed at the top of these basic blocks? Should .cfi_def_cfa_register %rbp be placed as well? If you move these basic blocks around, .cfi_def_cfa_register %rbp is currently not tracked.

That is because .cfi_def_cfa %rbp, 16 is identical to the following:
.cfi_def_cfa_register %rbp
.cfi_def_cfa_offset 16

Honestly I am not a CFI expert but I have read enough bits of LLVM libunwind and am not completely CFI illiterate (I have fixed a very subtle negative cfiDefCfa bug). The description of the patch is still puzzling me.
I think it lacks a summary about what the patch intends to do.

Is the intention: if the entry block stays in the front of the function, while other basic blocks can be randomly shuffled => the CFI states of all basic blocks are the same no matter how non-entry basic blocks are shuffled?

Or

The entry basic block can be shuffled as well?

We explicitly want to support the case where all BBs can be shuffled around arbitrarily.

Fri, Jul 10, 2:44 PM · Restricted Project

Oct 18 2018

amharc updated the diff for D51936: Fix a use-after-RAUW bug in large GEP splitting.

Updated the patch to manually merge vectors of large GEP offsets when doing a RAUW of an invariant.group strip/launder intrinsic. This requires only handling keys and not values of the map, as values are always GetElementPtr instructions and not intrinsic calls.

Oct 18 2018, 6:31 PM

Sep 12 2018

amharc added reviewers for D51936: Fix a use-after-RAUW bug in large GEP splitting: haicheng, efriedma.
Sep 12 2018, 11:49 PM

Sep 11 2018

amharc added a comment to D51936: Fix a use-after-RAUW bug in large GEP splitting.

The other AssertingVHs held by large GEP splitting code are either AssertingVH<GetElementPtrInst> or refer to possibly bitcasted GetElementPtrInsts. As far as I can see, there is no RAUWing of those being done in CGP at the moment.

Sep 11 2018, 9:09 AM
amharc created D51936: Fix a use-after-RAUW bug in large GEP splitting.
Sep 11 2018, 9:02 AM

Jul 3 2018

amharc added inline comments to D47423: Simplify recursive launder.invariant.group and strip.
Jul 3 2018, 2:15 AM

May 29 2018

amharc accepted D47108: [CodeGenCXX] Add -fforce-emit-vtables.

Looks good to me. Obviously, you should wait for someone more competent than me to accept it, too.

May 29 2018, 12:24 AM

May 28 2018

amharc requested changes to D47108: [CodeGenCXX] Add -fforce-emit-vtables.
May 28 2018, 4:04 AM

May 27 2018

amharc added a comment to D47423: Simplify recursive launder.invariant.group and strip.

Otherwise looks good to me. Obviously, you should wait for someone more competent than me to approve it ;)

May 27 2018, 9:40 AM

May 21 2018

amharc added inline comments to D47103: Implement strip.invariant.group.
May 21 2018, 9:13 PM
amharc added a comment to D47108: [CodeGenCXX] Add -fforce-emit-vtables.

I think that MarkVTableUsed should be called somewhere in Sema (possibly ActOnFinishCXXMemberDecls?) if ForceEmitVTables is on. This probably requires making ForceEmitVTables a LangOption in addition to it being a CodeGenOption.

May 21 2018, 2:02 PM
amharc requested changes to D47108: [CodeGenCXX] Add -fforce-emit-vtables.

This is not sound: sometimes the forcefully emitted vtable is incorrect due to destructor aliasing. This happens e.g. in the Bullet benchmark from the llvm test suite. A simplified example follows.

May 21 2018, 5:12 AM

May 19 2018

amharc accepted D47088: Fix aliasing of launder.invariant.group.

Of course, you should wait for an LGTM from someone more competent than me ;)

May 19 2018, 5:00 AM

May 16 2018

amharc added a comment to D46900: [BasicAA] Fix handling of invariant group launders.

Removed unnecessary { }.

May 16 2018, 3:26 AM
amharc updated the diff for D46900: [BasicAA] Fix handling of invariant group launders.

Added a direct AA test and cautionary comments in both BasicAA and CaptureTracking, removed unnecessary { }.

May 16 2018, 3:25 AM

May 15 2018

amharc added reviewers for D46900: [BasicAA] Fix handling of invariant group launders: kuhar, rsmith.
May 15 2018, 1:13 PM
amharc created D46900: [BasicAA] Fix handling of invariant group launders.
May 15 2018, 1:10 PM

May 5 2018

amharc added inline comments to D32423: Constant fold launder of null and undef.
May 5 2018, 5:03 AM

May 3 2018

amharc added a comment to D32673: [CaptureTracking] Handle capturing of launder.invariant.group.

Otherwise looks good to me.

May 3 2018, 4:46 AM
amharc accepted D45419: Dissallow non-empty metadata for invariant.group.

Looks good to me.

May 3 2018, 4:33 AM
amharc accepted D45320: [MemDep] Fixed handling of invariant.group.

I think it would be a little bit cleaner if the relevant checks were added to verifyRemoved too, where all other maps are traversed in debug builds. However, AssertingVH is probably a good enough guarantee ;)

May 3 2018, 4:25 AM

May 1 2018

amharc added inline comments to D45111: Rename invariant.group.barrier to launder.invariant.group.
May 1 2018, 2:10 PM

Apr 5 2018

amharc added a comment to D45320: [MemDep] Fixed handling of invariant.group.

Would handling NonLocalDefsCache in the same places as other DenseMaps from Instruction * be sufficient? In particular, RemoveCachedNonLocalPointerDependencies, called by both removeInstruction and invalidateCachedPointerInfo, looks promising ;)

Apr 5 2018, 12:41 PM
amharc added inline comments to D45150: Less conservative LoopSafetyInfo for headers.
Apr 5 2018, 10:00 AM · Restricted Project
amharc added a comment to D45320: [MemDep] Fixed handling of invariant.group.

I am not sure that ValueMap can be moveable: if it was moved, the ValueMapT *Map pointer in ValueMapCallbackVH could become invalid.

Apr 5 2018, 9:52 AM
amharc added inline comments to D45320: [MemDep] Fixed handling of invariant.group.
Apr 5 2018, 6:50 AM

Mar 31 2017

amharc added inline comments to D31531: Remove readnone from invariant.group.barrier.
Mar 31 2017, 10:17 AM