Page MenuHomePhabricator

aqjune (Juneyoung Lee)
User

Projects

User does not belong to any projects.

User Details

User Since
Jan 17 2017, 8:49 PM (279 w, 3 d)

Recent Activity

Tue, May 24

aqjune accepted D126296: [ValueTracking] Loads with !dereferenceable metadata cannot be undef/poison.

Per LangRef statements of the definition of dereferenceable(<n>) attribute:

The pointer should be well defined, otherwise it is undefined behavior. This means dereferenceable(<n>) implies noundef.

Tue, May 24, 8:13 AM · Restricted Project, Restricted Project

Wed, May 18

aqjune accepted D125869: [JumpThreading] Insert freeze when unfolding select.

LGTM

Wed, May 18, 4:58 PM · Restricted Project, Restricted Project

Tue, May 17

aqjune committed rG85fb9ccfa387: Precommit a test file for D84941 (authored by aqjune).
Precommit a test file for D84941
Tue, May 17, 6:52 PM · Restricted Project, Restricted Project
aqjune committed rG3adcf96b4faa: [JumpThreading] Let ProcessImpliedCondition look into freeze instructions (authored by aqjune).
[JumpThreading] Let ProcessImpliedCondition look into freeze instructions
Tue, May 17, 6:51 PM · Restricted Project, Restricted Project
aqjune closed D84941: [JumpThreading] Let ProcessImpliedCondition look into freeze instructions.
Tue, May 17, 6:51 PM · Restricted Project, Restricted Project
aqjune updated the diff for D84941: [JumpThreading] Let ProcessImpliedCondition look into freeze instructions.

Make test show diff, add a description about why @and_noopt is not optimized yet, update comments in ProcessImpliedCondition

Tue, May 17, 6:28 PM · Restricted Project, Restricted Project

Wed, May 11

aqjune added inline comments to D125398: [ControlHeightReduction] Freeze condition when converting select to branch.
Wed, May 11, 9:15 PM · Restricted Project, Restricted Project

Mon, May 2

aqjune added a comment to D122835: [SCEV] Removed an unnecessary assertion in SCEV..

Hi, sorry for my delay. The old assertion seems problematic.

Mon, May 2, 10:33 PM · Restricted Project, Restricted Project
aqjune added a comment to D123473: [LICM] Only create load in pre-header when promoting load..

I don't think we actually want to do that. Instead, I think we're making a mistake by considering the load to be immediate UB as opposed to simply returning poison in this case.

Mon, May 2, 6:42 AM · Restricted Project, Restricted Project

Sun, May 1

aqjune added a comment to D124526: [SimpleLoopUnswitch] Collect either logical ANDs/ORs but not both..

There was a similar miscompilation report (which was introduced by my patch) and a bug fix due to the existence of select cond, true, false in the past: https://reviews.llvm.org/rG5bb38e84d3d0#986154 and https://reviews.llvm.org/rG6b4b1dc6ec6f0bf0a1bb414fbe751ccab99d41a0
Also a follow-up patch to enhance an assertion condition: https://reviews.llvm.org/rG431a40e1e28f181e87dd247b91a5e6872dd64ab4

This only fixes trivial unswitching for now, but a similar problem
likely exists with non-trivial unswitching.

If the select cond, true, false is a concern, what do you think about simplifying it to cond (https://alive2.llvm.org/ce/z/p5Ahrp) in advance before loop unswitch?

Is it possible you did not check the latest version of the patch? It uses helpers to skip select cons, true, false when trying to match logical and/or.

I don't think we really should simplify/modify the IR if we are not unswitching, which is what 6b4b1dc6ec6f0bf0a1bb414fbe751ccab99d41a0 did, but SimpleLoopUnswitch claims no changes have been made.

Sun, May 1, 12:52 AM · Restricted Project, Restricted Project

Sat, Apr 30

aqjune added a comment to D124677: [ConstantFold] Don't convert getelementptr to ptrtoint+inttoptr.

I don't think this follows: This is talking about "integer constant expressions", which are a front-end concern. It means that the front-end is required to match for (void*)0 and emit that as ptr null rather than inttoptr (i64 0 to ptr). At least from that wording, doing int x = 0; (void*)xdoes not result in a null pointer (though possibly other wording implies that?)

I don't think having ptrtoint 1 have universal provenance and ptrtoint 0 have nullary provenance can lead to consistent semantics. It renders many transforms that are "obviously correct" illegal, such as:

define ptr @src(ptr %p, i64 %idx) {
  %p2 = getelementptr i8, ptr %p, i64 %idx
  ret ptr %p2
}
define ptr @tgt(ptr %p, i64 %idx) {
  %p.int = ptrtoint ptr %p to i64
  %p.add = add i64 %p.int, %idx
  %p2 = inttoptr i64 %p.add to ptr
  ret ptr %p2
}

While this transform is very undesirable, it should be correct because it only increases provenance. However, due to the special ptrtoint 0 handling this is incorrect for the special case where p.int == -idx.

Sat, Apr 30, 9:07 PM · Restricted Project, Restricted Project
aqjune committed rG40a2e35599b5: [InstCombine] Remove the undef-related workaround code in visitSelectInst (authored by aqjune).
[InstCombine] Remove the undef-related workaround code in visitSelectInst
Sat, Apr 30, 4:52 AM · Restricted Project, Restricted Project
aqjune closed D124426: [InstCombine] Remove the undef-related workaround code in visitSelectInst.
Sat, Apr 30, 4:52 AM · Restricted Project, Restricted Project
aqjune added a comment to D124677: [ConstantFold] Don't convert getelementptr to ptrtoint+inttoptr.

This has been observed as a real-world miscompile with rustc.

Sat, Apr 30, 3:48 AM · Restricted Project, Restricted Project
aqjune added a comment to D124677: [ConstantFold] Don't convert getelementptr to ptrtoint+inttoptr.

I agree that introducing ptrtoint + inttoptr here doesn't sound like a good idea because both it is bad for alias analysis and its correctness is not clear.

Sat, Apr 30, 3:36 AM · Restricted Project, Restricted Project
aqjune added a comment to D124526: [SimpleLoopUnswitch] Collect either logical ANDs/ORs but not both..

There was a similar miscompilation report (which was introduced by my patch) and a bug fix due to the existence of select cond, true, false in the past: https://reviews.llvm.org/rG5bb38e84d3d0#986154 and https://reviews.llvm.org/rG6b4b1dc6ec6f0bf0a1bb414fbe751ccab99d41a0
Also a follow-up patch to enhance an assertion condition: https://reviews.llvm.org/rG431a40e1e28f181e87dd247b91a5e6872dd64ab4

Sat, Apr 30, 3:30 AM · Restricted Project, Restricted Project

Apr 27 2022

aqjune updated the diff for D124426: [InstCombine] Remove the undef-related workaround code in visitSelectInst.

Yep, a test added

Apr 27 2022, 3:09 AM · Restricted Project, Restricted Project

Apr 25 2022

aqjune requested review of D124426: [InstCombine] Remove the undef-related workaround code in visitSelectInst.
Apr 25 2022, 6:07 PM · Restricted Project, Restricted Project

Apr 23 2022

aqjune added a comment to D124321: [InstSimplify] Use canReplacePointersIfEqual to conditionally simplify '(ptr1 == ptr2) ? a : b'.

haveSameProvenanceIfEqual seems like a better name, certainly.

Apr 23 2022, 7:48 PM · Restricted Project, Restricted Project
aqjune added a comment to D124321: [InstSimplify] Use canReplacePointersIfEqual to conditionally simplify '(ptr1 == ptr2) ? a : b'.

I began to think that we might not need to make replacement of pointers 'directional' because it is too complex.
What about renaming canReplacePointersIfEqual in Loads.h to haveSameProvenance and only checking the equivalence of two pointers, which will make things simpler (including this patch)?

Apr 23 2022, 7:24 PM · Restricted Project, Restricted Project
aqjune added a comment to D124252: [SimpleLoopUnswitch] Enable freezing of conditions by default..

It sounds great - then after the removal of the old pass I will make a patch that removes https://github.com/llvm/llvm-project/blob/main/llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp#L2508 as well.

Apr 23 2022, 6:53 PM · Restricted Project, Restricted Project
aqjune updated subscribers of D124252: [SimpleLoopUnswitch] Enable freezing of conditions by default..
Apr 23 2022, 1:34 AM · Restricted Project, Restricted Project
aqjune added a comment to D124252: [SimpleLoopUnswitch] Enable freezing of conditions by default..

A quick question: should LoopUnswitch.cpp be fixed as well? It seems the pass is not maintained though. Will SimpleLoopUnswitch supercede the LoopUnswitch pass?

Apr 23 2022, 1:33 AM · Restricted Project, Restricted Project
aqjune added inline comments to D124321: [InstSimplify] Use canReplacePointersIfEqual to conditionally simplify '(ptr1 == ptr2) ? a : b'.
Apr 23 2022, 1:25 AM · Restricted Project, Restricted Project
aqjune requested review of D124321: [InstSimplify] Use canReplacePointersIfEqual to conditionally simplify '(ptr1 == ptr2) ? a : b'.
Apr 23 2022, 1:24 AM · Restricted Project, Restricted Project

Apr 22 2022

aqjune accepted D124252: [SimpleLoopUnswitch] Enable freezing of conditions by default..

Thank you for your work..! The diff looks fine.

Apr 22 2022, 8:19 AM · Restricted Project, Restricted Project

Apr 21 2022

aqjune added a comment to D123991: [LangRef] Clarify load/store of non-byte-sized types.

Well, here we are discussing the semantics of LLVM IR. It's ok for the semantics of SDAG to be different, as long as it's a refinement of LLVM IR's.

Apr 21 2022, 6:46 PM · Restricted Project, Restricted Project

Apr 19 2022

aqjune added a comment to D123991: [LangRef] Clarify load/store of non-byte-sized types.

I confirmed that there are a few other architectures assuming the same semantics:

Apr 19 2022, 6:19 PM · Restricted Project, Restricted Project

Mar 1 2022

aqjune added a comment to D120647: [CGP] Sink compare through freeze.

Also, could you leave a link to Alive2 in the patch description showing that pushing freeze into icmp ops is safe when there are multiple users?

Mar 1 2022, 12:32 AM · Restricted Project, Restricted Project
aqjune added inline comments to D120647: [CGP] Sink compare through freeze.
Mar 1 2022, 12:08 AM · Restricted Project, Restricted Project

Jan 26 2022

aqjune added a comment to D89050: Add support for !noundef metatdata on loads.

Right, but the absence of select -> and/or folding by itself shouldn't matter -- it's only relevant in that it can prevent other transforms

I fully agree, it's just that the presence of that transformation was helping with an optimization later on in the pipeline
in my specific case.

Otherwise there is no issue with code generation of select.

Jan 26 2022, 5:23 PM · Restricted Project

Jan 7 2022

aqjune added a comment to D116766: [SCEV] Sequential/in-order `UMin` expression.

Since x == 0 ? x : umin(x, y) cannot be represented using the current SCEV operations (at least using the ops in SCEVTypes). I believe the new ops in this patch are necessary.
Another approach to support such expressions would be adding a ternary operator and comparisons to SCEV, but it would require bigger changes, I guess?

Jan 7 2022, 1:13 AM · Restricted Project

Jan 5 2022

aqjune added a comment to D93793: [IR] Let IRBuilder's CreateVectorSplat/CreateShuffleVector use poison as placeholder.

I have a babyism question, why poison is preferred to the undef in the pattern ConstantVector::getSplat ?

Jan 5 2022, 6:31 AM · Restricted Project, Restricted Project

Dec 15 2021

aqjune added a comment to D115804: [CodeGen] use saturating FP casts when compiling with "no-strict-float-cast-overflow".

This sounds like a great fix to me! :)

Dec 15 2021, 5:15 PM · Restricted Project

Dec 13 2021

aqjune added a comment to D115526: [InstCombine] don't automatically drop poison-generating flags in SimplifyVectorDemandedElts.

It looks like the shufflevector poison change hasn't landed yet? D93818 / D103874

Thanks for the reminder! ( cc @aqjune @hyeongyukim @regehr )

Dec 13 2021, 6:18 PM · Restricted Project

Nov 25 2021

aqjune committed rG35c1e6ac1af0: [MLIR] [docs] Fix misguided examples in memref.subview operation. (authored by makesource).
[MLIR] [docs] Fix misguided examples in memref.subview operation.
Nov 25 2021, 4:25 AM
aqjune closed D114500: [MLIR] [docs] Fix misguided examples in memref.subview operation..
Nov 25 2021, 4:24 AM · Restricted Project

Nov 12 2021

aqjune added a comment to D113543: [RISCV] Add inline expansion for vector ftrunc/fceil/ffloor..

Hello all,
I couldn't find this mail because it was somehow buried in other mails.

Nov 12 2021, 9:39 PM · Restricted Project

Nov 5 2021

aqjune added a reverting change for D105169: [Clang/Test]: Rename enable_noundef_analysis to disable-noundef-analysis and turn it off by default: rG89ad2822affb: Revert "[Clang/Test]: Rename enable_noundef_analysis to disable-noundef….
Nov 5 2021, 11:40 PM · Restricted Project
aqjune added a reverting change for rG7584ef766a72: [Clang/Test]: Rename enable_noundef_analysis to disable-noundef-analysis and…: rG89ad2822affb: Revert "[Clang/Test]: Rename enable_noundef_analysis to disable-noundef….
Nov 5 2021, 11:40 PM
aqjune committed rG89ad2822affb: Revert "[Clang/Test]: Rename enable_noundef_analysis to disable-noundef… (authored by aqjune).
Revert "[Clang/Test]: Rename enable_noundef_analysis to disable-noundef…
Nov 5 2021, 11:40 PM
aqjune committed rG7584ef766a72: [Clang/Test]: Rename enable_noundef_analysis to disable-noundef-analysis and… (authored by aqjune).
[Clang/Test]: Rename enable_noundef_analysis to disable-noundef-analysis and…
Nov 5 2021, 11:37 PM

Nov 3 2021

aqjune added a comment to D105169: [Clang/Test]: Rename enable_noundef_analysis to disable-noundef-analysis and turn it off by default.

You are absolutely right. X86 variant uses an "=a" constraint (rax register), others pin the output variable to a specific register with asm declaration. It appears we've missed it in Aarch64.

Could you check if asm("x0") in the declaration of res helps?

Nov 3 2021, 1:40 AM · Restricted Project

Nov 1 2021

aqjune added a comment to D105169: [Clang/Test]: Rename enable_noundef_analysis to disable-noundef-analysis and turn it off by default.

I am not familiar with inline assembly, but it seems the output variable (%0) is not updated because it does not appear in the code.

Nov 1 2021, 8:53 PM · Restricted Project

Oct 28 2021

aqjune added inline comments to D92270: [ConstantFold] Fold more operations to poison.
Oct 28 2021, 8:19 PM · Restricted Project, Restricted Project

Oct 18 2021

aqjune committed rGf193bcc701de: Revert D105169 due to the two-stage failure in ASAN (authored by aqjune).
Revert D105169 due to the two-stage failure in ASAN
Oct 18 2021, 7:53 AM
aqjune added a reverting change for D105169: [Clang/Test]: Rename enable_noundef_analysis to disable-noundef-analysis and turn it off by default: rGf193bcc701de: Revert D105169 due to the two-stage failure in ASAN.
Oct 18 2021, 7:53 AM · Restricted Project
aqjune added a comment to D105169: [Clang/Test]: Rename enable_noundef_analysis to disable-noundef-analysis and turn it off by default.

I will revert this patch, since its fix needs some time for me to investigate.
I have a colleague who has access to an AArch64 server, so I can give it a try by myself.

Oct 18 2021, 7:02 AM · Restricted Project
aqjune added a comment to D105169: [Clang/Test]: Rename enable_noundef_analysis to disable-noundef-analysis and turn it off by default.

Yes, I believe so. If the branch is not taken, pos_min and pos_max are undefined when entering ff_gen_search. (I would assume that their value isn't ever used within ff_gen_search in that case.) But regardless of that, in this case, the generated code crashes around this line, https://gist.github.com/aqjune/3bd0ea19bbc12b4744843c0c070e994c#file-ff_seek_frame_binary-c-L39, before entering ff_gen_search - and within that branch, those variables are properly set before they're used.

Oct 18 2021, 1:58 AM · Restricted Project

Oct 17 2021

aqjune added a comment to D105169: [Clang/Test]: Rename enable_noundef_analysis to disable-noundef-analysis and turn it off by default.

It seems the original code has a use of an uninitialized variable.
Line 4420 at seek-preproc.c (function ff_seek_frame_binary):

 int64_t pos_min=pos_min, pos_max=pos_max, pos, pos_limit; // pos_min and pos_max are self-assigned.
...
if (sti->index_entries) {
   ...
}
// pos_min and pos_max are used as arguments below
pos = ff_gen_search(s, stream_index, target_ts, pos_min, pos_max, pos_limit,
                     ts_min, ts_max, flags, &ts, avif->read_timestamp);
Oct 17 2021, 8:25 PM · Restricted Project
aqjune added a comment to D105169: [Clang/Test]: Rename enable_noundef_analysis to disable-noundef-analysis and turn it off by default.

I can see that @ff_seek_frame_binary is the only affected function.
It introduces llvm.assume as well as !nonnull at a few places and folds null pointer checks.
Still investigating..

Oct 17 2021, 6:47 PM · Restricted Project

Oct 16 2021

aqjune committed rG37ca7a795b27: Fix missing failures in clang-ppc64be* and retry fixing clang-x64-windows-msvc (authored by aqjune).
Fix missing failures in clang-ppc64be* and retry fixing clang-x64-windows-msvc
Oct 16 2021, 12:20 AM

Oct 15 2021

aqjune committed rG9aa6c72b92b6: Fix lit test failures in clang-ppc* and clang-x64-windows-msvc (authored by aqjune).
Fix lit test failures in clang-ppc* and clang-x64-windows-msvc
Oct 15 2021, 10:34 PM
aqjune committed rG705387c5074b: Resolve lit failures in clang after 8ca4b3e's land (authored by aqjune).
Resolve lit failures in clang after 8ca4b3e's land
Oct 15 2021, 9:52 PM
aqjune added a comment to D105169: [Clang/Test]: Rename enable_noundef_analysis to disable-noundef-analysis and turn it off by default.

There has been no concern in a week, so I'd like to land this patch and D108453 in this weekend.
I'll carefully watch the buildbots and address failures if any.
@eugenis could you accept this patch and D108453 please, if you don't mind?

Oct 15 2021, 1:26 AM · Restricted Project

Oct 12 2021

aqjune added a comment to D111515: [InstCombine] Remove attributes after hoisting free above null check.

I thought about the validity of this transformation.
To be pedantic, the validity is unclear when ptr is
(1) an out-of-bounds pointer such that (intptr_t)ptr is 0.
But, I remember people wanted to regard free(ptr) as equivalent to free(null) in that case, so it would be fine.
(2) a partially undefined pointer s.t. ptr = undef | 1
But... I couldn't see any such expression appear in practice, and I don't think LLVM is strictly correct with respect to partially undef pointers

Oct 12 2021, 6:07 PM · Restricted Project
aqjune added a comment to D111643: [ValueTracking] Let propgatesPoison consider single poison operand..

The change looks great to me.

Oct 12 2021, 7:05 AM · Restricted Project

Oct 7 2021

aqjune added a comment to D105169: [Clang/Test]: Rename enable_noundef_analysis to disable-noundef-analysis and turn it off by default.

Thank you!

Oct 7 2021, 6:16 PM · Restricted Project

Sep 29 2021

aqjune added inline comments to rG6b4b1dc6ec6f: [LoopUnswitch] Simplify branch condition if it is select with constant operands.
Sep 29 2021, 6:49 PM

Sep 28 2021

aqjune accepted D110311: [IR] Change the default value of InstertElement to poison (1/4).

LGTM

Sep 28 2021, 2:29 AM · Restricted Project

Sep 27 2021

aqjune added a comment to D110311: [IR] Change the default value of InstertElement to poison (1/4).

@spatel Thanks!
@hyeongyukim Do you mind if we add the new CreateInsertElements and use them in this patch?

Sep 27 2021, 5:08 PM · Restricted Project

Sep 26 2021

aqjune updated the diff for D93818: [LangRef] Update shufflevector's semantics to return poison if the mask is undef.

Explicitly state that undef can be given as a shuffle mask and its meaning is equivalent to that of the poison mask

Sep 26 2021, 7:27 AM · Restricted Project
aqjune added inline comments to D103874: [IR] Rename the shufflevector's undef mask to poison.
Sep 26 2021, 7:04 AM · Restricted Project, Restricted Project
aqjune updated the diff for D103874: [IR] Rename the shufflevector's undef mask to poison.

Resurrect mistakenly removed test statements

Sep 26 2021, 6:36 AM · Restricted Project, Restricted Project
aqjune updated the diff for D103874: [IR] Rename the shufflevector's undef mask to poison.

Rebase

Sep 26 2021, 6:20 AM · Restricted Project, Restricted Project
aqjune added inline comments to D93818: [LangRef] Update shufflevector's semantics to return poison if the mask is undef.
Sep 26 2021, 6:04 AM · Restricted Project

Sep 25 2021

aqjune abandoned D94380: [InstCombine] Update transformations to use poison for insertelement/shufflevector's placeholder value (1/2).

I abandon this patch because (1) shufflevector parts are covered in D110226, D110227, D110230, and (2) insertelement parts will be covered in upcoming new patches.

Sep 25 2021, 8:14 PM · Restricted Project, Restricted Project
aqjune abandoned D93817: [InstCombine] Update transformations to use poison for insertelement/shufflevector's placeholder value (2/2).

I abandon this patch because (1) shufflevector parts are covered in D110226, D110227, D110230, and (2) insertelement parts will be covered in upcoming new patches.

Sep 25 2021, 8:12 PM · Restricted Project, Restricted Project
aqjune added a comment to D110311: [IR] Change the default value of InstertElement to poison (1/4).

I left comments on a few changes - I think it is okay to leave ones that are clearly valid if you prefer.

Sep 25 2021, 8:09 PM · Restricted Project

Sep 20 2021

aqjune added inline comments to D93818: [LangRef] Update shufflevector's semantics to return poison if the mask is undef.
Sep 20 2021, 1:57 AM · Restricted Project

Aug 22 2021

aqjune added a comment to D108453: [Clang/Test]: Rename enable_noundef_analysis to disable-noundef-analysis and turn it off by default (2).

Hello all, I'm a messenger of @hyeongyukim's two patches that allow clang to turn on noundef analysis by default.
The motivation and background of these patches are described at https://reviews.llvm.org/D105169#2959464.

Aug 22 2021, 11:55 PM · Restricted Project, Restricted Project
aqjune added a comment to D105169: [Clang/Test]: Rename enable_noundef_analysis to disable-noundef-analysis and turn it off by default.

Hello all,

Aug 22 2021, 11:45 PM · Restricted Project
aqjune retitled D108453: [Clang/Test]: Rename enable_noundef_analysis to disable-noundef-analysis and turn it off by default (2) from [Clang/Test]: Enable enable_noundef_analysis as default(2) to [Clang/Test]: Rename enable_noundef_analysis to disable-noundef-analysis and turn it off by default (2).
Aug 22 2021, 11:23 PM · Restricted Project, Restricted Project
aqjune retitled D105169: [Clang/Test]: Rename enable_noundef_analysis to disable-noundef-analysis and turn it off by default from [Clang/Test]: Enable enable_noundef_analysis by default to [Clang/Test]: Rename enable_noundef_analysis to disable-noundef-analysis and turn it off by default.
Aug 22 2021, 11:22 PM · Restricted Project
aqjune added reviewers for D105169: [Clang/Test]: Rename enable_noundef_analysis to disable-noundef-analysis and turn it off by default: guiand, eugenis.
Aug 22 2021, 11:21 PM · Restricted Project
aqjune added reviewers for D108453: [Clang/Test]: Rename enable_noundef_analysis to disable-noundef-analysis and turn it off by default (2): guiand, eugenis.
Aug 22 2021, 11:20 PM · Restricted Project, Restricted Project
aqjune updated the diff for D108453: [Clang/Test]: Rename enable_noundef_analysis to disable-noundef-analysis and turn it off by default (2).

Rebase, rename disable-noundef-args to disable-noundef-analysis, remove redundant diffs

Aug 22 2021, 10:41 PM · Restricted Project, Restricted Project
aqjune updated the diff for D105169: [Clang/Test]: Rename enable_noundef_analysis to disable-noundef-analysis and turn it off by default.

Rename disable-noundef-args to disable-noundef-analysis

Aug 22 2021, 10:38 PM · Restricted Project

Aug 21 2021

aqjune added a comment to D108504: [InstCombine][NFC] refactor roundtripcast.

In terms of correctness, I think the patch looks okay.

Aug 21 2021, 1:57 AM · Restricted Project

Aug 20 2021

aqjune added inline comments to D107822: [InstCombine] Fold Int2Ptr/PtrToInt if the ptr is dereferenceable.
Aug 20 2021, 7:35 PM · Restricted Project

Aug 19 2021

aqjune added inline comments to D107822: [InstCombine] Fold Int2Ptr/PtrToInt if the ptr is dereferenceable.
Aug 19 2021, 9:16 PM · Restricted Project
aqjune added a comment to D107822: [InstCombine] Fold Int2Ptr/PtrToInt if the ptr is dereferenceable.

I became to wonder what happens if a pointer is freed in the function.
For example:

// assume that %p is dereferenceable(8)
call void @g(%p);
%q = inttoptr (ptrtoint %p);
use(%q); // can we replace this with %p?
Aug 19 2021, 12:11 AM · Restricted Project

Aug 9 2021

aqjune accepted D107751: [IR] Let ConstantVector::getSplat use poison instead of undef.

LGTM, thank you for writing this patch.
Since the poison values in the insertelement and shufflevector are not visible from shufflevector's users, I believe this change is harmless.

Aug 9 2021, 7:09 AM · Restricted Project

Aug 8 2021

aqjune added inline comments to D107580: [VectorCombine] Support AND/UREM indices that require freezing..
Aug 8 2021, 5:39 PM · Restricted Project

Aug 3 2021

aqjune added a comment to D106041: [SimpleLoopUnswitch] Re-fix introduction of UB when hoisted condition may be undef or poison.

I think so? it sounds great.
Could you try inserting freeze to icmp only when one of its operands is constant?
ex) freeze (icmp op0, 1) -> icmp (freeze(op0), 1)
ex2) freeze (icmp op0, op1) -/-> icmp (freeze(op0), freeze(op1)) <= this splits one freeze into two; its benefit is not clear.

Aug 3 2021, 6:36 AM · Restricted Project

Aug 2 2021

aqjune added inline comments to D106289: [InstCombine] Fold phi ( inttoptr/ptrtoint x ) to phi (x).
Aug 2 2021, 9:20 PM · Restricted Project
aqjune added a comment to D106289: [InstCombine] Fold phi ( inttoptr/ptrtoint x ) to phi (x).

I left a few nitpicks that is probably worth addressed

Aug 2 2021, 6:36 AM · Restricted Project
aqjune accepted D106289: [InstCombine] Fold phi ( inttoptr/ptrtoint x ) to phi (x).

LGTM.

Aug 2 2021, 6:26 AM · Restricted Project

Jul 30 2021

aqjune added a comment to D106779: [SimplifyCFG] Make ValueEqaulityComparison freeze-aware..

If loop unswitch introduced the freeze, what about adding an extra InstCombine pass between the loop unswitch and SimplifyCFG?

Jul 30 2021, 5:36 PM · Restricted Project
aqjune added a comment to D107180: [InstCombine] Prevent unnecessary sinks caused by the Freeze.

As suggested, finding a common dominator and sinking the instruction to the location seems reasonable and worth trying.
It will help reduce the diff at line 3866~, I hope.
If DT is null, fallback to the old one use check?

Jul 30 2021, 5:35 PM · Restricted Project
aqjune abandoned D88432: [InstCombine] Fix pr47668.

Just found that this old patch is still open - I'd like to revisit this later since freeze can be used to resurrect this transformation. This will require investigation into the impact of freeze.

Jul 30 2021, 5:22 PM · Restricted Project
aqjune accepted D107174: [DAG] isGuaranteedNotToBeUndefOrPoison - handle ISD::BUILD_VECTOR nodes.

LGTM too.

Jul 30 2021, 4:54 PM · Restricted Project
aqjune added a comment to D106289: [InstCombine] Fold phi ( inttoptr/ptrtoint x ) to phi (x).

I have a question about addrspacecast: is ptrtoint(inttoptr(ptrtoint pty1 p) to pty2) equivalent to ptrtoint(addrspacecast pty1 p to pty2)?
Does anyone have idea about this?

I'm going to guess no. inttoptr/ptrtoint are essentially no-op bitcasts,
while addrspacecast is implementation-defined and in general case can be assumed to e.g. offset the pointer.

Jul 30 2021, 4:52 AM · Restricted Project
aqjune added a comment to D106289: [InstCombine] Fold phi ( inttoptr/ptrtoint x ) to phi (x).

I have a question about addrspacecast: is ptrtoint(inttoptr(ptrtoint pty1 p) to pty2) equivalent to ptrtoint(addrspacecast pty1 p to pty2)?
Does anyone have idea about this?

Jul 30 2021, 4:38 AM · Restricted Project

Jul 27 2021

aqjune abandoned D101185: [LangRef] tbaa: type names can be used as hints to optimizations.
Jul 27 2021, 5:38 PM · Restricted Project
aqjune abandoned D100717: [InstCombine] Transform memcpy to ptr load/stores if TBAA says so.

Since there is a second opinion about using TBAA, will close this for now.

Jul 27 2021, 5:34 PM · Restricted Project
aqjune committed rG4f71f59bf3d9: [DAGCombiner] Fold SETCC(FREEZE(x),const) to FREEZE(SETCC(x,const)) if SETCC is… (authored by aqjune).
[DAGCombiner] Fold SETCC(FREEZE(x),const) to FREEZE(SETCC(x,const)) if SETCC is…
Jul 27 2021, 5:23 PM
aqjune committed rG784f258c0970: Precommit test files for D105344 (NFC) (authored by aqjune).
Precommit test files for D105344 (NFC)
Jul 27 2021, 5:23 PM
aqjune closed D105344: [DAGCombiner] Fold SETCC(FREEZE(x),const) to FREEZE(SETCC(x,const)) if SETCC is used by BRCOND.
Jul 27 2021, 5:22 PM · Restricted Project
aqjune added inline comments to D105344: [DAGCombiner] Fold SETCC(FREEZE(x),const) to FREEZE(SETCC(x,const)) if SETCC is used by BRCOND.
Jul 27 2021, 12:38 AM · Restricted Project