Page MenuHomePhabricator

aqjune (Juneyoung Lee)
User

Projects

User does not belong to any projects.

User Details

User Since
Jan 17 2017, 8:49 PM (220 w, 4 d)

Recent Activity

Thu, Apr 8

aqjune added inline comments to D100122: Update m_Undef to match vectors/aggrs with undefs and poisons mixed.
Thu, Apr 8, 9:12 AM · Restricted Project
aqjune added a comment to D99853: [InstSimplify] Teach isUndefValue to understand const vector with both undef & poison.

I made https://reviews.llvm.org/D100122 to show how the patch would look like if m_Undef is fixed instead.

Thu, Apr 8, 9:10 AM · Restricted Project
aqjune requested review of D100122: Update m_Undef to match vectors/aggrs with undefs and poisons mixed.
Thu, Apr 8, 9:09 AM · Restricted Project

Wed, Apr 7

aqjune committed rGfe16aa7d6551: [Constant] Remove unused variable (authored by aqjune).
[Constant] Remove unused variable
Wed, Apr 7, 11:45 PM
aqjune committed rG648544f998cd: [Constant] ConstantStruct/Array should not lower poison to undef (authored by aqjune).
[Constant] ConstantStruct/Array should not lower poison to undef
Wed, Apr 7, 11:23 PM
aqjune added a comment to D99853: [InstSimplify] Teach isUndefValue to understand const vector with both undef & poison.

This problem isn't really limited to this one function in InstSimplify. Might it make sense to adjust UndefValue to actually represent such constants? We'd have to add a bitmask to it that indicates which values are poison.

Wed, Apr 7, 10:39 PM · Restricted Project
aqjune added a comment to D82317: [Clang/Test]: Update tests where `noundef` attribute is necessary.

Thank you all! Incremental change makes sense to me as well. It will help smooth landing without buildbot failures.
I'll start writing patches soon.

Wed, Apr 7, 5:28 PM · Restricted Project
aqjune accepted D99884: [LV] Logical and/or select costs.

LGTM, Thanks!

Wed, Apr 7, 5:20 PM · Restricted Project
aqjune added a comment to D99642: For non-null pointer checks, do not descend through out-of-bounds GEPs.

LGTM, thanks!

@nlopes do you have any more thoughts on the difference between LLVM & Alive2 on this topic?

Wed, Apr 7, 4:20 PM · Restricted Project

Mon, Apr 5

aqjune added a comment to D99814: [JumpThreading] Change asserts for WantInteger into actual checks.

This tracks down to a commit in 2010: https://github.com/llvm/llvm-project/commit/d9df6eaa9ce20
Interestingly the motivation of introducing ConstantPreference was to properly deal with indirectbr. :)
I have no strong preference either.

Mon, Apr 5, 7:18 PM · Restricted Project
aqjune added inline comments to D99884: [LV] Logical and/or select costs.
Mon, Apr 5, 6:29 PM · Restricted Project

Sun, Apr 4

aqjune added a comment to D96945: [InstCombine] Add simplification of two logical and/ors.

This turns out to be a bug in InstCombine. I reported it as https://llvm.org/pr49832 .

Sun, Apr 4, 6:43 AM · Restricted Project

Sat, Apr 3

aqjune committed rG5207cde5cb41: [InstCombine] Conditionally fold select i1 into and/or (authored by aqjune).
[InstCombine] Conditionally fold select i1 into and/or
Sat, Apr 3, 10:16 PM
aqjune closed D99674: [InstCombine] Conditionally fold select i1 into and/or.
Sat, Apr 3, 10:16 PM · Restricted Project
aqjune added a comment to D93990: [InstSimplify] Return poison if insertelement touches out of bounds.

Hi! It looks like this may be causing https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=29445 .

Sat, Apr 3, 10:14 PM · Restricted Project
aqjune added inline comments to D99853: [InstSimplify] Teach isUndefValue to understand const vector with both undef & poison.
Sat, Apr 3, 10:10 PM · Restricted Project
aqjune updated the diff for D99674: [InstCombine] Conditionally fold select i1 into and/or.

rebase, clang-format

Sat, Apr 3, 10:08 PM · Restricted Project
aqjune requested review of D99853: [InstSimplify] Teach isUndefValue to understand const vector with both undef & poison.
Sat, Apr 3, 9:57 PM · Restricted Project
aqjune committed rG6147501617f0: [InstSimplify] Add a test for folding comparison with a undef vector (NFC) (authored by aqjune).
[InstSimplify] Add a test for folding comparison with a undef vector (NFC)
Sat, Apr 3, 9:47 PM
aqjune added a comment to D99674: [InstCombine] Conditionally fold select i1 into and/or.

I'm OK with landing this first to fix an active miscompile, but I don't think we should block flipping the switch on noundef work. I think your numbers support that we shouldn't wait on that, because the noundef impact it small relative to the whole change (it only makes a 15% difference in affected files).

Sat, Apr 3, 9:35 PM · Restricted Project
aqjune committed rG732a90da785d: [InstCombine] precommit pr49688.ll (NFC) (authored by aqjune).
[InstCombine] precommit pr49688.ll (NFC)
Sat, Apr 3, 9:30 PM
aqjune committed rGf1d4af4058e8: [InstCombine] Reapply update_test_checks.py to unsigned-multiply-overflow-check. (authored by aqjune).
[InstCombine] Reapply update_test_checks.py to unsigned-multiply-overflow-check.
Sat, Apr 3, 9:28 PM
aqjune added inline comments to D99481: [InstCombine] Fix miscompile on GEP+load to icmp fold (PR45210).
Sat, Apr 3, 11:08 AM · Restricted Project

Thu, Apr 1

aqjune committed rGc6647693300b: [AssumeBundles] offset should be added to correctly calculate align (authored by aqjune).
[AssumeBundles] offset should be added to correctly calculate align
Thu, Apr 1, 8:32 PM
aqjune closed D98759: [AssumeBundles] offset should be added to correctly calculate align.
Thu, Apr 1, 8:32 PM · Restricted Project
aqjune updated the diff for D99674: [InstCombine] Conditionally fold select i1 into and/or.

rebase, correctify the test file name

Thu, Apr 1, 9:08 AM · Restricted Project
aqjune added a comment to D99674: [InstCombine] Conditionally fold select i1 into and/or.

Unconditionally disabling this transformation affects later pipelines that depend on and/or i1s.
To minimize its impact, this patch conservatively checks whether cond2 is an instruction that
creates a poison or its operand creates a poison.

Do you have any further information on the regressions this would result in?

Given all the preliminary work that has gone into this, I'd really like to have the option flipped outright, and not introduce another hack.

Thu, Apr 1, 9:06 AM · Restricted Project
aqjune added a comment to D96945: [InstCombine] Add simplification of two logical and/ors.

It indeed seems tricky to find out the root cause for this; this optimization enabled vectorization which led to the error for some reason.
I'll have more time to investigate this tomorrow and weekends.

Thu, Apr 1, 7:47 AM · Restricted Project

Wed, Mar 31

aqjune committed rGdf0b97dab08a: [ValueTracking] Add with.overflow intrinsics to poison analysis functions (authored by aqjune).
[ValueTracking] Add with.overflow intrinsics to poison analysis functions
Wed, Mar 31, 10:42 AM
aqjune closed D99671: [ValueTracking] Add with.overflow intrinsics to poison analysis functions.
Wed, Mar 31, 10:42 AM · Restricted Project
aqjune added inline comments to D99674: [InstCombine] Conditionally fold select i1 into and/or.
Wed, Mar 31, 10:40 AM · Restricted Project
aqjune requested review of D99674: [InstCombine] Conditionally fold select i1 into and/or.
Wed, Mar 31, 10:37 AM · Restricted Project
aqjune updated the summary of D99671: [ValueTracking] Add with.overflow intrinsics to poison analysis functions.
Wed, Mar 31, 10:08 AM · Restricted Project
aqjune requested review of D99671: [ValueTracking] Add with.overflow intrinsics to poison analysis functions.
Wed, Mar 31, 10:03 AM · Restricted Project

Tue, Mar 30

aqjune added a comment to rG5bb38e84d3d0: [LoopUnswitch] unswitch if cond is in select form of and/or as well.

I made a fix to keep the main branch in a good state, but not sure this is a real solution because there might be a hidden problem. I'm investigating it.

Please add an assertion that m_LogicalOr() and m_LogicalAnd() aren't true at the same time.

Tue, Mar 30, 6:38 PM
aqjune committed rG431a40e1e28f: [LoopUnswitch] Assert that branch condition is either and/or but not both (authored by aqjune).
[LoopUnswitch] Assert that branch condition is either and/or but not both
Tue, Mar 30, 6:35 PM
aqjune added a comment to rG5bb38e84d3d0: [LoopUnswitch] unswitch if cond is in select form of and/or as well.

I made a fix to keep the main branch in a good state, but not sure this is a real solution because there might be a hidden problem. I'm investigating it.

Tue, Mar 30, 4:14 AM
aqjune committed rG6b4b1dc6ec6f: [LoopUnswitch] Simplify branch condition if it is select with constant operands (authored by aqjune).
[LoopUnswitch] Simplify branch condition if it is select with constant operands
Tue, Mar 30, 4:10 AM

Mon, Mar 29

aqjune added a comment to rG5bb38e84d3d0: [LoopUnswitch] unswitch if cond is in select form of and/or as well.

I'll make a fix for this, thanks.

Mon, Mar 29, 6:48 PM
aqjune added a comment to D82317: [Clang/Test]: Update tests where `noundef` attribute is necessary.

Hello all,
Is there a plan to enable -enable-noundef-analysis by default? It seems this patch is already addressing a lot of test cases.
Another good news is that DeadArgElim is also fixed by D98899 to correctly drop UB-raising attributes such as noundef when replacing an unused argument with undef. This was an issue which is known to be problematic when the flag was activated.
I can make a patch based on this work instead if people want.

Mon, Mar 29, 9:19 AM · Restricted Project

Sat, Mar 27

aqjune committed rG05884d3b525a: Make FoldBranchToCommonDest poison-safe by default (authored by aqjune).
Make FoldBranchToCommonDest poison-safe by default
Sat, Mar 27, 3:05 AM
aqjune closed D99452: Make FoldBranchToCommonDest poison-safe by default.
Sat, Mar 27, 3:05 AM · Restricted Project
aqjune updated the summary of D99452: Make FoldBranchToCommonDest poison-safe by default.
Sat, Mar 27, 3:04 AM · Restricted Project
aqjune added inline comments to D99452: Make FoldBranchToCommonDest poison-safe by default.
Sat, Mar 27, 2:40 AM · Restricted Project
aqjune updated the summary of D99452: Make FoldBranchToCommonDest poison-safe by default.
Sat, Mar 27, 1:52 AM · Restricted Project
aqjune requested review of D99452: Make FoldBranchToCommonDest poison-safe by default.
Sat, Mar 27, 1:51 AM · Restricted Project

Fri, Mar 26

aqjune committed rGfc3f0c9cc085: [IRCE] Use m_LogicalAnd (authored by aqjune).
[IRCE] Use m_LogicalAnd
Fri, Mar 26, 11:23 PM
aqjune added a comment to D98684: [LangRef] state that align assume op bundle may take an extra argument.

gently ping

Fri, Mar 26, 7:14 PM · Restricted Project
aqjune added a comment to D98759: [AssumeBundles] offset should be added to correctly calculate align.

ping

Fri, Mar 26, 7:13 PM · Restricted Project

Wed, Mar 24

aqjune added a comment to D99303: Reword lifetime description to avoid contradicting long term implementation.

The core notions here are: 1) separate dereferenceability and object lifetime (in the marker intrinsic sense), and 2) unify handling for all memory object types.

Wed, Mar 24, 9:13 PM · Restricted Project

Tue, Mar 23

aqjune committed rG960a7673683f: Reland "[InstCombine] Add simplification of two logical and/ors" (authored by aqjune).
Reland "[InstCombine] Add simplification of two logical and/ors"
Tue, Mar 23, 12:25 AM

Mon, Mar 22

aqjune abandoned D98585: [InstSimplify] Update foldings to constants to consider Q.AllowRefinement.

Closed in favor of D99027

Mon, Mar 22, 5:34 PM · Restricted Project
aqjune abandoned D98391: [InstSimplify] Propagate AllowRefinement info from SimplifyWithOpReplaced.

Closed in favor of D99027

Mon, Mar 22, 5:33 PM · Restricted Project
aqjune committed rG5c2e50b5d241: Reland "[SimplifyCFG] Update FoldBranchToCommonDest to be poison-safe" (authored by aqjune).
Reland "[SimplifyCFG] Update FoldBranchToCommonDest to be poison-safe"
Mon, Mar 22, 5:20 PM
aqjune added a comment to D93065: [InstCombine] Disable optimizations of select instructions that causes propagation of poison values.

Should be fixed via https://reviews.llvm.org/rGb00209ed100c

Mon, Mar 22, 2:02 PM · Restricted Project, Restricted Project
aqjune committed rGb00209ed100c: [SCEV] Use logical and/or matcher (authored by aqjune).
[SCEV] Use logical and/or matcher
Mon, Mar 22, 2:01 PM

Sun, Mar 21

aqjune added inline comments to D99027: [InstCombine] Whitelist non-refining folds in SimplifyWithOpReplaced.
Sun, Mar 21, 7:51 PM · Restricted Project

Sat, Mar 20

aqjune accepted D99027: [InstCombine] Whitelist non-refining folds in SimplifyWithOpReplaced.

It is impressive that leaving the few patterns was enough to cover all existing unit tests.
If more patterns are needed, we can factor out the non-refined foldings from SimplifyXXInst as a static function and call it here.

Sat, Mar 20, 8:01 PM · Restricted Project
aqjune committed rG319d093b87a8: [CFLGraph] Fix a crash due to missing handling of freeze (authored by aqjune).
[CFLGraph] Fix a crash due to missing handling of freeze
Sat, Mar 20, 2:15 AM

Fri, Mar 19

aqjune added a comment to D85534: Enable InsertFreeze flag of JumpThreading when used in LTO.
In D85534#2636321, @Jim wrote:

Hi,
This flag is enabled in LTO. I met a failure on my test program.
It shows

Unsupported instruction encountered
llvm/lib/Analysis/CFLGraph.h:260!

It looks like that CFLGraph is unable to support FreezeInst.
Could you give me some suggesion to fix it?
Thanks

Fri, Mar 19, 1:42 AM · Restricted Project

Wed, Mar 17

aqjune added a comment to D98391: [InstSimplify] Propagate AllowRefinement info from SimplifyWithOpReplaced.

In terms of maintenance: if reviewers think the complexity this patch is introducing is big to maintain, I'll turn this patch to using canCreatePoison check since it will fix the bug anyway.

Wed, Mar 17, 6:26 AM · Restricted Project
aqjune added a comment to D98391: [InstSimplify] Propagate AllowRefinement info from SimplifyWithOpReplaced.

Does this direction look okay? @nikic
One benefit of this approach is that it detects unsafe replacements that have non-poison-creating instructions as well.
For example, this transformation is illegal:

  %v = select (x == 0), 0, (x & y)
=>
  %v = x & y

If x = 0 and y was poison this makes the result more undefined. However, the current check relying on canCreatePoison cannot detect this.
D98585 fixes this by making the relevant transformation respect Q.AllowRefinement.

Wed, Mar 17, 6:17 AM · Restricted Project

Tue, Mar 16

aqjune updated the summary of D98759: [AssumeBundles] offset should be added to correctly calculate align.
Tue, Mar 16, 11:56 PM · Restricted Project
aqjune requested review of D98759: [AssumeBundles] offset should be added to correctly calculate align.
Tue, Mar 16, 10:04 PM · Restricted Project
aqjune added a comment to D98684: [LangRef] state that align assume op bundle may take an extra argument.

the fact that we now have "multiple kinds of arguments" is confusing.

Tue, Mar 16, 6:36 PM · Restricted Project
aqjune updated the diff for D98684: [LangRef] state that align assume op bundle may take an extra argument.

update

Tue, Mar 16, 6:33 PM · Restricted Project
aqjune updated the diff for D98684: [LangRef] state that align assume op bundle may take an extra argument.

Address comments, minor update to the grammar

Tue, Mar 16, 6:31 PM · Restricted Project
aqjune added inline comments to D97244: [SimplifyCFG] Update passingValueIsAlwaysUndefined to check more attributes.
Tue, Mar 16, 6:08 PM · Restricted Project
aqjune added inline comments to D97244: [SimplifyCFG] Update passingValueIsAlwaysUndefined to check more attributes.
Tue, Mar 16, 5:23 PM · Restricted Project
aqjune updated the diff for D98684: [LangRef] state that align assume op bundle may take an extra argument.

update

Tue, Mar 16, 1:12 AM · Restricted Project
aqjune requested review of D98684: [LangRef] state that align assume op bundle may take an extra argument.
Tue, Mar 16, 1:11 AM · Restricted Project

Mon, Mar 15

aqjune committed rGedf634ebc267: [AssumeBundles] Add nonnull/align to op bundle if noundef exists (authored by aqjune).
[AssumeBundles] Add nonnull/align to op bundle if noundef exists
Mon, Mar 15, 10:24 AM
aqjune closed D98228: [AssumeBundles] Add nonnull/align to op bundle if noundef exists.
Mon, Mar 15, 10:24 AM · Restricted Project
aqjune added a comment to D98228: [AssumeBundles] Add nonnull/align to op bundle if noundef exists.

Thank you all!

Mon, Mar 15, 10:23 AM · Restricted Project
aqjune added inline comments to D98605: [LibCalls] Add `nosync` to known library calls.
Mon, Mar 15, 10:16 AM · Restricted Project
aqjune added inline comments to D98228: [AssumeBundles] Add nonnull/align to op bundle if noundef exists.
Mon, Mar 15, 10:09 AM · Restricted Project
aqjune updated the diff for D98228: [AssumeBundles] Add nonnull/align to op bundle if noundef exists.

check if the argument has noundef

Mon, Mar 15, 10:06 AM · Restricted Project
aqjune added inline comments to D98605: [LibCalls] Add `nosync` to known library calls.
Mon, Mar 15, 5:45 AM · Restricted Project

Sun, Mar 14

aqjune added a comment to D98605: [LibCalls] Add `nosync` to known library calls.

To be safe, what do you think about marking nosync to ops that can be represented as a series of loads/stores or scalar ops only? For example, I believe memset is nosync because it is equivalent to a series of nonatomic stores.
For side-effecting operations, such as printf, I'm not 100% sure whether it is nosync. printf interacts with cout to properly flush buffers, which might do some interactions.

Sun, Mar 14, 7:58 PM · Restricted Project
aqjune updated the summary of D98391: [InstSimplify] Propagate AllowRefinement info from SimplifyWithOpReplaced.
Sun, Mar 14, 7:00 AM · Restricted Project
aqjune updated the diff for D98391: [InstSimplify] Propagate AllowRefinement info from SimplifyWithOpReplaced.

Make computePointerICmp slightly more conservative if Q.AllowRefinement is false

Sun, Mar 14, 6:58 AM · Restricted Project

Sat, Mar 13

aqjune added a comment to D98228: [AssumeBundles] Add nonnull/align to op bundle if noundef exists.

I think the process of checking is a value is noundef needs to be more general than just calls. maybe addKnowledge would be a better place.
because nonnull and align information can also be inferred from loads/stores.

Sat, Mar 13, 9:42 AM · Restricted Project
aqjune requested review of D98585: [InstSimplify] Update foldings to constants to consider Q.AllowRefinement.
Sat, Mar 13, 9:25 AM · Restricted Project
aqjune updated the diff for D98391: [InstSimplify] Propagate AllowRefinement info from SimplifyWithOpReplaced.

minor updates

Sat, Mar 13, 8:36 AM · Restricted Project
aqjune updated the diff for D98391: [InstSimplify] Propagate AllowRefinement info from SimplifyWithOpReplaced.

Add AllowRefinement field

Sat, Mar 13, 8:34 AM · Restricted Project
aqjune retitled D98391: [InstSimplify] Propagate AllowRefinement info from SimplifyWithOpReplaced from [InstSimplify] Set UseInstrInfo to false when calling SimplifyWithOpReplaced to [InstSimplify] Propagate AllowRefinement info from SimplifyWithOpReplaced.
Sat, Mar 13, 8:34 AM · Restricted Project
aqjune added a comment to D98391: [InstSimplify] Propagate AllowRefinement info from SimplifyWithOpReplaced.

TBH, I don't feel comfortable with extending the canCreatePoison() check. :( Even if it requires inspection of foldings in InstSimplify, I think a safer way is to add a flag that disallows foldings returning refined values.
Let me update this patch and write following patches for updating them.

Sat, Mar 13, 6:57 AM · Restricted Project

Mar 11 2021

aqjune accepted D98410: [ConstantFold] Handle undef/poison when constant folding smul_fix/smul_fix_sat.
Mar 11 2021, 9:14 AM · Restricted Project
aqjune added inline comments to D98410: [ConstantFold] Handle undef/poison when constant folding smul_fix/smul_fix_sat.
Mar 11 2021, 7:59 AM · Restricted Project
aqjune added a comment to D98410: [ConstantFold] Handle undef/poison when constant folding smul_fix/smul_fix_sat.

Wouldn't folding C * undef and undef * C into an undef, rather than 0, be valid/better? Is there a reason we cannot do that?

I figure that it would be wrong if for example C is zero, because then the result can't take any other value than zero. Or if C is 2 and scale is 0, then the result can't be odd. So undef include values that isn't possible for every combination of the other operands.

The poison propagation is a bit more new for me. I hope I got that part correct.

Mar 11 2021, 6:33 AM · Restricted Project
aqjune added a comment to D90529: Allow nonnull/align attribute to accept poison.

A fix in AssumeBundleBuilder to make it comply LangRef: D98228

BTW, I found that "align" can take two operands: "align"(i8* ptr, i64 a, i64 b) What is the meaning of the second index (b)?

the meaning of the second index is the offset of the alignment. so with "align"(i8* ptr, i64 16, i64 12), ptr is aligned on 4 but ptr + 4 is aligned on 16.
I introduced this to support __builtin_assume_aligned which has similar semantics.

Mar 11 2021, 6:18 AM · Restricted Project

Mar 10 2021

aqjune added a comment to D98391: [InstSimplify] Propagate AllowRefinement info from SimplifyWithOpReplaced.

Confirmed that this resolves the regression (https://reviews.llvm.org/D95026#2610334) from two-stage build!

Mar 10 2021, 10:45 PM · Restricted Project
aqjune added a comment to D96663: [InstCombine] Fold icmp (select c,const,arg), null if arg has nonnullattr.

ping

Mar 10 2021, 9:43 PM · Restricted Project
aqjune abandoned D97454: [IR] Add CallSiteOnly flag to CallBase's attribute getters.

Well, I found a way to get around this issue without changing these APIs. :)

Mar 10 2021, 9:41 PM · Restricted Project
aqjune updated the diff for D98391: [InstSimplify] Propagate AllowRefinement info from SimplifyWithOpReplaced.

Fix deoptimized case

Mar 10 2021, 9:28 PM · Restricted Project
aqjune added inline comments to D98391: [InstSimplify] Propagate AllowRefinement info from SimplifyWithOpReplaced.
Mar 10 2021, 9:22 PM · Restricted Project
aqjune requested review of D98391: [InstSimplify] Propagate AllowRefinement info from SimplifyWithOpReplaced.
Mar 10 2021, 9:19 PM · Restricted Project
aqjune committed rG720a828045e1: Resolve unused variable warning (NFC) (authored by aqjune).
Resolve unused variable warning (NFC)
Mar 10 2021, 7:04 PM
aqjune committed rG8652c3e1a373: [InstSimplify] Pass SimplifyQuery to computePointerICmp (NFC) (authored by aqjune).
[InstSimplify] Pass SimplifyQuery to computePointerICmp (NFC)
Mar 10 2021, 6:14 PM
aqjune committed rG317097817325: [InstSimplify] Add tests for pr49495 (NFC) (authored by aqjune).
[InstSimplify] Add tests for pr49495 (NFC)
Mar 10 2021, 12:55 AM