[PATCH] D152658: [InstCombine] Change SimplifyDemandedVectorElts to use PoisonElts instead of UndefElts

2023-07-11 Thread Juneyoung Lee via Phabricator via cfe-commits
aqjune added inline comments. Comment at: llvm/lib/Transforms/InstCombine/InstructionCombining.cpp:1653 - if (ShMask[I] >= 0) { -assert(ShMask[I] < (int)NumElts && "Not expecting narrowing shuffle"); Constant *NewCElt = NewVecC[ShMask[I]]; a

[PATCH] D152658: [InstCombine] Change SimplifyDemandedVectorElts to use PoisonElts instead of UndefElts

2023-07-11 Thread Juneyoung Lee via Phabricator via cfe-commits
aqjune added a comment. Hi, thanks for your hard work! Comment at: llvm/lib/Transforms/InstCombine/InstructionCombining.cpp:1653 - if (ShMask[I] >= 0) { -assert(ShMask[I] < (int)NumElts && "Not expecting narrowing shuffle"); Constant *NewCElt = NewVecC[ShM

[PATCH] D144903: [X86] Drop single use check for freeze(undef) in LowerAVXCONCAT_VECTORS

2023-03-08 Thread Juneyoung Lee via Phabricator via cfe-commits
aqjune added inline comments. Comment at: clang/test/CodeGen/X86/avx-cast-builtins.c:1 // RUN: %clang_cc1 %s -O3 -flax-vector-conversions=none -ffreestanding %s -triple=x86_64-unknown-unknown -target-feature +avx -target-feature +avx512f -target-feature +avx512fp16 -S -o - |

[PATCH] D144903: [X86] Drop single use check for freeze(undef) in LowerAVXCONCAT_VECTORS

2023-03-08 Thread Juneyoung Lee via Phabricator via cfe-commits
aqjune added inline comments. Comment at: clang/test/CodeGen/X86/avx-cast-builtins.c:1 // RUN: %clang_cc1 %s -O3 -flax-vector-conversions=none -ffreestanding %s -triple=x86_64-unknown-unknown -target-feature +avx -target-feature +avx512f -target-feature +avx512fp16 -S -o - |

[PATCH] D143287: [Clang][X86] Change X86 cast intrinsics to use __builtin_nondeterministic_value

2023-03-08 Thread Juneyoung Lee via Phabricator via cfe-commits
aqjune added a comment. In D143287#4179344 , @ManuelJBrito wrote: > In D143287#4179020 , @aqjune wrote: > >> H, is D104790 superseded by this patch? > > I don't think so we stil

[PATCH] D143287: [Clang][X86] Change X86 cast intrinsics to use __builtin_nondeterministic_value

2023-03-08 Thread Juneyoung Lee via Phabricator via cfe-commits
aqjune added a comment. H, is D104790 superseded by this patch? I wonder what is the status of this patch as well. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143287/new/ https://reviews.llvm.org/D143287 _

[PATCH] D136737: [Draft] [clang] Add builtin_unspecified_value

2023-02-22 Thread Juneyoung Lee via Phabricator via cfe-commits
aqjune abandoned this revision. aqjune added a comment. Closing this as D142388 added a function that can be used instead Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136737/new/ https://reviews.llvm.org/D136737

[PATCH] D136737: [Draft] [clang] Add builtin_unspecified_value

2022-10-25 Thread Juneyoung Lee via Phabricator via cfe-commits
aqjune added a comment. The updates are analogous to how `GNUNullExprClass` is processed because `UnspecifiedValueExprClass` and it is similar (have no operand). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136737/new/ https://reviews.llvm.org/D1

[PATCH] D136737: [Draft] [clang] Add builtin_unspecified_value

2022-10-25 Thread Juneyoung Lee via Phabricator via cfe-commits
aqjune created this revision. Herald added subscribers: steakhal, martong, arphaman. Herald added a reviewer: shafik. Herald added a reviewer: NoQ. Herald added a project: All. aqjune requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. This pat

[PATCH] D103874: [IR] Rename the shufflevector's undef mask to poison

2022-08-10 Thread Juneyoung Lee via Phabricator via cfe-commits
aqjune added inline comments. Comment at: clang/test/CodeGen/X86/avx-builtins.c:182 // CHECK-LABEL: test_mm256_castsi128_si256 - // CHECK: shufflevector <2 x i64> %{{.*}}, <2 x i64> %{{.*}}, <4 x i32> + // CHECK: shufflevector <2 x i64> %{{.*}}, <2 x i64> %{{.*}}, <4 x i32>

[PATCH] D103874: [IR] Rename the shufflevector's undef mask to poison

2022-06-27 Thread Juneyoung Lee via Phabricator via cfe-commits
aqjune added a comment. PR31524 (https://github.com/llvm/llvm-project/issues/31524) discusses about the lowering of such intrinsics. According to the PR, it seems users consider undefined intrinsics as returning unknown but consistent bits. If it works, my updates in the draft (https://github.

[PATCH] D103874: [IR] Rename the shufflevector's undef mask to poison

2022-06-27 Thread Juneyoung Lee via Phabricator via cfe-commits
aqjune added a comment. In D103874#3611483 , @nikic wrote: > Which intrinsic are you working on here? If this is about the mm_undefined > intrinsics, why do we need to change those from the current status quo of > using a zero value instead of undef? I

[PATCH] D103874: [IR] Rename the shufflevector's undef mask to poison

2022-06-27 Thread Juneyoung Lee via Phabricator via cfe-commits
aqjune added a comment. In D103874#3594617 , @aqjune wrote: > It seems llvm/lib/Target/X86/X86ISelLowering.cpp's LowerAVXCONCAT_VECTORS is > relevant to efficient lowering of `shufflevector %x, freeze(poison), mask`. After patching `LowerAVXCONCAT_VECTO

[PATCH] D103874: [IR] Rename the shufflevector's undef mask to poison

2022-06-19 Thread Juneyoung Lee via Phabricator via cfe-commits
aqjune added a comment. Herald added subscribers: jsji, kosarev. Herald added a project: All. It seems llvm/lib/Target/X86/X86ISelLowering.cpp's LowerAVXCONCAT_VECTORS is relevant to efficient lowering of `shufflevector %x, freeze(poison), mask`. Repository: rG LLVM Github Monorepo CHANGES S

[PATCH] D93793: [IR] Let IRBuilder's CreateVectorSplat/CreateShuffleVector use poison as placeholder

2022-01-05 Thread Juneyoung Lee via Phabricator via cfe-commits
aqjune added a comment. In D93793#3221414 , @Allen wrote: > I have a babyism question, why poison is preferred to the undef in the > pattern ConstantVector::getSplat ? Hi Allen, It is because folding poison is easier than folding undef. :) For example, a

[PATCH] D115804: [CodeGen] use saturating FP casts when compiling with "no-strict-float-cast-overflow"

2021-12-15 Thread Juneyoung Lee via Phabricator via cfe-commits
aqjune added a comment. This sounds like a great fix to me! :) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D115804/new/ https://reviews.llvm.org/D115804 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/m

[PATCH] D105169: [Clang/Test]: Rename enable_noundef_analysis to disable-noundef-analysis and turn it off by default

2021-11-03 Thread Juneyoung Lee via Phabricator via cfe-commits
aqjune added a comment. In D105169#3104262 , @eugenis wrote: > 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 i

[PATCH] D105169: [Clang/Test]: Rename enable_noundef_analysis to disable-noundef-analysis and turn it off by default

2021-11-01 Thread Juneyoung Lee via Phabricator via cfe-commits
aqjune added a comment. 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. 1382 __asm__ __volatile__( 1383"mov x0,x2\n" /* flags */ 1384"mov x2,x4\n" /* pt

[PATCH] D92270: [ConstantFold] Fold more operations to poison

2021-10-28 Thread Juneyoung Lee via Phabricator via cfe-commits
aqjune added inline comments. Comment at: llvm/lib/IR/ConstantFold.cpp:633 // the input constant. -return UndefValue::get(DestTy); +return PoisonValue::get(DestTy); } neildhar wrote: > spatel wrote: > > MatzeB wrote: > > > I believ

[PATCH] D105169: [Clang/Test]: Rename enable_noundef_analysis to disable-noundef-analysis and turn it off by default

2021-10-18 Thread Juneyoung Lee via Phabricator via cfe-commits
aqjune added a comment. I will revert this patch, since its fix needs some time for me to investigate. I have access to an AArch server, so I can give it a try by myself. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D105169/new/ https://reviews.llv

[PATCH] D105169: [Clang/Test]: Rename enable_noundef_analysis to disable-noundef-analysis and turn it off by default

2021-10-18 Thread Juneyoung Lee via Phabricator via cfe-commits
aqjune added a comment. In D105169#3069417 , @mstorsjo wrote: > 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_searc

[PATCH] D105169: [Clang/Test]: Rename enable_noundef_analysis to disable-noundef-analysis and turn it off by default

2021-10-17 Thread Juneyoung Lee via Phabricator via cfe-commits
aqjune added a comment. 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) {

[PATCH] D105169: [Clang/Test]: Rename enable_noundef_analysis to disable-noundef-analysis and turn it off by default

2021-10-17 Thread Juneyoung Lee via Phabricator via cfe-commits
aqjune added a comment. 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.. F19677416: before.ll F19677415: after.ll

[PATCH] D105169: [Clang/Test]: Rename enable_noundef_analysis to disable-noundef-analysis and turn it off by default

2021-10-15 Thread Juneyoung Lee via Phabricator via cfe-commits
aqjune added a comment. 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

[PATCH] D105169: [Clang/Test]: Rename enable_noundef_analysis to disable-noundef-analysis and turn it off by default

2021-10-07 Thread Juneyoung Lee via Phabricator via cfe-commits
aqjune added a comment. Thank you! I'll send a mail to cfe-dev + llvm-dev to notify the change (and gather any possible concern if exists). If there is no objection in a week, let's land these patches. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org

[PATCH] D103874: [IR] Rename the shufflevector's undef mask to poison

2021-09-26 Thread Juneyoung Lee via Phabricator via cfe-commits
aqjune marked an inline comment as done. aqjune added inline comments. Comment at: clang/test/CodeGen/X86/avx-builtins.c:182 // CHECK-LABEL: test_mm256_castsi128_si256 - // CHECK: shufflevector <2 x i64> %{{.*}}, <2 x i64> %{{.*}}, <4 x i32> + // CHECK: shufflevector <2 x i

[PATCH] D103874: [IR] Rename the shufflevector's undef mask to poison

2021-09-26 Thread Juneyoung Lee via Phabricator via cfe-commits
aqjune updated this revision to Diff 375095. aqjune added a comment. Resurrect mistakenly removed test statements Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D103874/new/ https://reviews.llvm.org/D103874 Files: clang/test/CodeGen/X86/avx-builti

[PATCH] D103874: [IR] Rename the shufflevector's undef mask to poison

2021-09-26 Thread Juneyoung Lee via Phabricator via cfe-commits
aqjune updated this revision to Diff 375094. aqjune added a comment. Herald added a subscriber: arphaman. Rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D103874/new/ https://reviews.llvm.org/D103874 Files: clang/test/CodeGen/X86/avx-builtin

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

2021-09-25 Thread Juneyoung Lee via Phabricator via cfe-commits
aqjune abandoned this revision. aqjune added a comment. I abandon this patch because (1) shufflevector parts are covered in D110226 , D110227 , D110230 , and (2) insertelement parts will be cov

[PATCH] D93817: [InstCombine] Update transformations to use poison for insertelement/shufflevector's placeholder value (2/2)

2021-09-25 Thread Juneyoung Lee via Phabricator via cfe-commits
aqjune abandoned this revision. aqjune added a comment. I abandon this patch because (1) shufflevector parts are covered in D110226 , D110227 , D110230 , and (2) insertelement parts will be cov

[PATCH] D108453: [Clang/Test]: Rename enable_noundef_analysis to disable-noundef-analysis and turn it off by default (2)

2021-08-23 Thread Juneyoung Lee via Phabricator via cfe-commits
aqjune added a comment. 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. Here is a list of tests that has `RUN: %clang_cc1`

[PATCH] D108453: [Clang/Test]: Enable enable_noundef_analysis as default(2)

2021-08-23 Thread Juneyoung Lee via Phabricator via cfe-commits
aqjune updated this revision to Diff 368034. aqjune added a comment. Rebase, rename `disable-noundef-args` to `disable-noundef-analysis`, remove redundant diffs Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D108453/new/ https://reviews.llvm.org/D10

[PATCH] D105169: [Clang/Test]: Rename enable_noundef_analysis to disable-noundef-analysis and turn it off by default

2021-08-22 Thread Juneyoung Lee via Phabricator via cfe-commits
aqjune added a comment. Hello all, I'm a messenger of @hyeongyukim's two patches that allow clang to turn on noundef analysis by default. The noundef analysis flag was added by D81678 in the past. Its goal is to mark arguments and return values in C/C++ as `no

[PATCH] D105169: [Clang/Test]: Enable enable_noundef_analysis by default

2021-08-22 Thread Juneyoung Lee via Phabricator via cfe-commits
aqjune updated this revision to Diff 368033. aqjune added a comment. Rename `disable-noundef-args` to `disable-noundef-analysis` Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D105169/new/ https://reviews.llvm.org/D105169 Files: clang/include/clan

[PATCH] D82317: [Clang/Test]: Update tests where `noundef` attribute is necessary

2021-06-30 Thread Juneyoung Lee via Phabricator via cfe-commits
aqjune added a comment. I asked @hyeongyukim (who is a participant of google summer of code) to rebase this patch and make an updated version. I thought it was a good goal for him to target, as the validity of the enable-noundef flag is already reviewed and the changes are syntactic updates to

[PATCH] D104790: [x86] fix mm*_undefined* intrinsics to use arbitrary frozen bit pattern

2021-06-27 Thread Juneyoung Lee via Phabricator via cfe-commits
aqjune added a comment. In D104790#2842523 , @nikic wrote: > Is this actually better in any meaningful way? InstCombine will turn `freeze > poison` into `zeroinitializer`, and until then this is just a completely > opaque value. I think to correctly em

[PATCH] D104790: [x86] fix mm*_undefined* intrinsics to use arbitrary frozen bit pattern

2021-06-26 Thread Juneyoung Lee via Phabricator via cfe-commits
aqjune added a comment. In D104790#2836463 , @craig.topper wrote: > In D104790#2836253 , @aqjune wrote: > >> I couldn't find end-to-end tests for checking assembly generation. >> To check whether this is working

[PATCH] D104790: [x86] fix mm*_undefined* intrinsics to use arbitrary frozen bit pattern

2021-06-26 Thread Juneyoung Lee via Phabricator via cfe-commits
aqjune updated this revision to Diff 354682. aqjune added a comment. - Update llvm/test/CodeGen/X86/sse-intrinsics-fast-isel.ll as well Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D104790/new/ https://reviews.llvm.org/D104790 Files: clang/lib/C

[PATCH] D104790: [x86] fix mm*_undefined* intrinsics to use arbitrary frozen bit pattern

2021-06-26 Thread Juneyoung Lee via Phabricator via cfe-commits
aqjune updated this revision to Diff 354678. aqjune added a comment. Minor fixes Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D104790/new/ https://reviews.llvm.org/D104790 Files: clang/lib/CodeGen/CGBuiltin.cpp clang/test/CodeGen/X86/avx-built

[PATCH] D104790: [x86] fix mm*_undefined* intrinsics to use arbitrary frozen bit pattern

2021-06-26 Thread Juneyoung Lee via Phabricator via cfe-commits
aqjune updated this revision to Diff 354675. aqjune added a comment. Herald added subscribers: llvm-commits, ecnelises, hiraditya. Herald added a project: LLVM. - Update llvm's fast-isels tests for undefined intrinsics to compile freeze(poison) - Update X86ISelLowering's getAVX2GatherNode and get

[PATCH] D104790: [x86] fix mm*_undefined* intrinsics to use arbitrary frozen bit pattern

2021-06-23 Thread Juneyoung Lee via Phabricator via cfe-commits
aqjune added a comment. I couldn't find end-to-end tests for checking assembly generation. To check whether this is working ok, which tests should I write and how would it look like? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D104790/new/ https:

[PATCH] D104790: [x86] fix mm*_undefined* intrinsics to use arbitrary frozen bit pattern

2021-06-23 Thread Juneyoung Lee via Phabricator via cfe-commits
aqjune created this revision. aqjune added reviewers: efriedma, spatel, craig.topper, RKSimon. Herald added a subscriber: pengfei. aqjune requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. This fixes lowering of `mm*_undefined*` intrinsics to

[PATCH] D103874: [IR] Rename the shufflevector's undef mask to poison

2021-06-08 Thread Juneyoung Lee via Phabricator via cfe-commits
aqjune added a comment. In D103874#2806519 , @efriedma wrote: > I noted the cases where it looks like the undef->poison change might actually > impact code using compiler intrinsic functions that have external > specifications. The relevant specificati

[PATCH] D103874: [IR] Rename the shufflevector's undef mask to poison

2021-06-08 Thread Juneyoung Lee via Phabricator via cfe-commits
aqjune created this revision. aqjune added reviewers: nikic, efriedma, spatel, fhahn, lebedev.ri, RKSimon. Herald added a reviewer: deadalnix. Herald added subscribers: dexonsmith, kerbowa, pengfei, dmgreen, zzheng, kbarton, hiraditya, nhaehnle, jvesely, nemanjai. aqjune requested review of this r

[PATCH] D101191: [InstCombine] Fully disable select to and/or i1 folding

2021-05-05 Thread Juneyoung Lee via Phabricator via cfe-commits
aqjune added a comment. In D101191#2738130 , @nikic wrote: > LGTM. I can take care of reverting if there are issues. Thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D101191/new/ https://reviews.llvm.

[PATCH] D101191: [InstCombine] Fully disable select to and/or i1 folding

2021-05-04 Thread Juneyoung Lee via Phabricator via cfe-commits
aqjune added a comment. I think this patch is ready to go: running the test-suite on an ARM machine didn't complain anything. Well, but one thing that I'm concerned about is that from tomorrow I'll not be online for about three weeks. :( I'd like to find someone who reverts this patch if there

[PATCH] D101191: [InstCombine] Fully disable select to and/or i1 folding

2021-05-03 Thread Juneyoung Lee via Phabricator via cfe-commits
aqjune added inline comments. Comment at: llvm/test/Transforms/InstCombine/or.ll:1135 ; CHECK-NEXT:ret i1 [[OR]] ; %x = icmp sge i16 %a, %b This can be salvaged as well: https://alive2.llvm.org/ce/z/yXF96T But I think there are more patterns that are m

[PATCH] D101191: [InstCombine] Fully disable select to and/or i1 folding

2021-05-03 Thread Juneyoung Lee via Phabricator via cfe-commits
aqjune added inline comments. Comment at: llvm/test/Transforms/InstCombine/or.ll:1102 +; CHECK-NEXT:[[AND:%.*]] = select i1 [[Y]], i1 [[X]], i1 false +; CHECK-NEXT:[[OR:%.*]] = select i1 [[X_INV]], i1 true, i1 [[AND]] ; CHECK-NEXT:ret i1 [[OR]] nikic

[PATCH] D101191: [InstCombine] Fully disable select to and/or i1 folding

2021-05-02 Thread Juneyoung Lee via Phabricator via cfe-commits
aqjune added a comment. (BTW, if the patch is reviewed, then I believe we are finally ready to land this patch.) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D101191/new/ https://reviews.llvm.org/D101191 __

[PATCH] D101191: [InstCombine] Fully disable select to and/or i1 folding

2021-05-02 Thread Juneyoung Lee via Phabricator via cfe-commits
aqjune added a comment. I made D101720 to cover the remaining cases except `Transforms/InstCombine/and2.ll`. Supporting `and2.ll` doesn't seem super-straightforward, but maybe some minor tweaks can make it. Repository: rG LLVM Github Monorepo CHANGES SINCE

[PATCH] D101191: [InstCombine] Fully disable select to and/or i1 folding

2021-05-01 Thread Juneyoung Lee via Phabricator via cfe-commits
aqjune added a comment. I found that there are a few more patterns that can be salvaged. Will create a patch for them. Comment at: llvm/test/Transforms/InstCombine/and.ll:898 ; CHECK-NEXT:[[Y:%.*]] = icmp ugt i72 [[C:%.*]], 42 -; CHECK-NEXT:[[AND:%.*]] = and i1 [[X]],

[PATCH] D101191: [InstCombine] Fully disable select to and/or i1 folding

2021-04-27 Thread Juneyoung Lee via Phabricator via cfe-commits
aqjune added inline comments. Comment at: llvm/test/Transforms/PhaseOrdering/unsigned-multiply-overflow-check.ll:20 +; FIXME: noundef should be attached to args define i1 @will_not_overflow(i64 %arg, i64 %arg1) { nikic wrote: > spatel wrote: > > aqjune wrote:

[PATCH] D101191: [InstCombine] Fully disable select to and/or i1 folding

2021-04-27 Thread Juneyoung Lee via Phabricator via cfe-commits
aqjune added inline comments. Comment at: llvm/test/Transforms/PhaseOrdering/unsigned-multiply-overflow-check.ll:20 +; FIXME: noundef should be attached to args define i1 @will_not_overflow(i64 %arg, i64 %arg1) { aqjune wrote: > spatel wrote: > > Any ideas ab

[PATCH] D101191: [InstCombine] Fully disable select to and/or i1 folding

2021-04-27 Thread Juneyoung Lee via Phabricator via cfe-commits
aqjune added inline comments. Comment at: llvm/test/Transforms/InstCombine/logical-select.ll:385 +; CHECK-NEXT:[[OR:%.*]] = select i1 [[AND1]], i1 true, i1 [[AND2]] +; CHECK-NEXT:ret i1 [[OR]] ; nikic wrote: > aqjune wrote: > > nikic wrote: > > > It look

[PATCH] D101191: [InstCombine] Fully disable select to and/or i1 folding

2021-04-23 Thread Juneyoung Lee via Phabricator via cfe-commits
aqjune added inline comments. Comment at: llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp:2680 +// select c, (select a, true, b), false -> select c, a, false +// if c implies that b is false. +if (match(CondVal, m_Select(m_Value(A), m_One(), m_Value(B))) && -

[PATCH] D101191: [InstCombine] Fully disable select to and/or i1 folding

2021-04-23 Thread Juneyoung Lee via Phabricator via cfe-commits
aqjune added inline comments. Comment at: llvm/test/Transforms/InstCombine/logical-select.ll:385 +; CHECK-NEXT:[[OR:%.*]] = select i1 [[AND1]], i1 true, i1 [[AND2]] +; CHECK-NEXT:ret i1 [[OR]] ; nikic wrote: > It looks like this fold could be salvaged, i

[PATCH] D82317: [Clang/Test]: Update tests where `noundef` attribute is necessary

2021-04-08 Thread Juneyoung Lee via Phabricator via cfe-commits
aqjune added a comment. 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. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82317/new/ https://reviews.l

[PATCH] D82317: [Clang/Test]: Update tests where `noundef` attribute is necessary

2021-03-29 Thread Juneyoung Lee via Phabricator via cfe-commits
aqjune added a comment. Herald added a subscriber: lxfind. 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

[PATCH] D81678: Introduce noundef attribute at call sites for stricter poison analysis

2021-02-09 Thread Juneyoung Lee via Phabricator via cfe-commits
aqjune added a comment. I'm fine with any direction that helps people land test updates/implementations. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D81678/new/ https://reviews.llvm.org/D81678 ___ cfe-c

[PATCH] D92270: [ConstantFold] Fold more operations to poison

2021-02-03 Thread Juneyoung Lee via Phabricator via cfe-commits
aqjune added a comment. In D92270#2538713 , @RKSimon wrote: > https://bugs.llvm.org/show_bug.cgi?id=49005 seems to be due to this (either > directly or it has unearthed an existing problem) I reverted this commit; I'll reland this after the underlying pr

[PATCH] D93817: [InstCombine] Update transformations to use poison for insertelement/shufflevector's placeholder value (2/2)

2021-01-10 Thread Juneyoung Lee via Phabricator via cfe-commits
aqjune added inline comments. Comment at: llvm/lib/Transforms/InstCombine/InstCombineSimplifyDemanded.cpp:1221 if (!isa(I->getOperand(1))) { -I->setOperand(1, UndefValue::get(I->getOperand(1)->getType())); MadeChange = true; This change is

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

2021-01-10 Thread Juneyoung Lee via Phabricator via cfe-commits
aqjune added inline comments. Comment at: llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp:1728 // Vec. if (IdxN % DstNumElts != 0 || IdxN + DstNumElts > VecNumElts) { replaceInstUsesWith(CI, UndefValue::get(CI.getType())); Guarded by t

[PATCH] D92270: [ConstantFold] Fold more operations to poison

2021-01-07 Thread Juneyoung Lee via Phabricator via cfe-commits
aqjune added a comment. In D92270#2483557 , @thakis wrote: > It turned out to be UB in our code as far as I can tell, see > https://bugs.chromium.org/p/angleproject/issues/detail?id=5500#c36 and the > follow-on. > > (If this is known to trigger an existi

[PATCH] D92270: [ConstantFold] Fold more operations to poison

2021-01-06 Thread Juneyoung Lee via Phabricator via cfe-commits
aqjune added a comment. To fix the old bug quite a few patches in LLVM have landed so far and it is still ongoing. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D92270/new/ https://reviews.llvm.org/D92270 __

[PATCH] D92270: [ConstantFold] Fold more operations to poison

2021-01-06 Thread Juneyoung Lee via Phabricator via cfe-commits
aqjune added a comment. In D92270#2482741 , @thakis wrote: > We bisected a test failure to this > (https://bugs.chromium.org/p/angleproject/issues/detail?id=5500#c17). Can you > expand a bit on what this patch means in practice? I'm guessing it makes UB

[PATCH] D93817: Update transformations to use poison for insertelement/shufflevector's placeholder value

2021-01-03 Thread Juneyoung Lee via Phabricator via cfe-commits
aqjune updated this revision to Diff 314289. aqjune added a comment. Rebase I'll continue splitting after working on lifetime patches Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93817/new/ https://reviews.llvm.org/D93817 Files: clang/test/Code

[PATCH] D93817: Update transformations to use poison for insertelement/shufflevector's placeholder value

2021-01-02 Thread Juneyoung Lee via Phabricator via cfe-commits
aqjune added a comment. I'll split this patch into smaller pieces, so they're able to get reviewed more easily. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93817/new/ https://reviews.llvm.org/D93817 _

[PATCH] D93817: Update transformations to use poison for insertelement/shufflevector's placeholder value

2020-12-31 Thread Juneyoung Lee via Phabricator via cfe-commits
aqjune added a comment. Running Alive2 finds one failure, which is fixed by updating the shufflevector semantics to return poison on undef/poison mask (D93818 ): Transforms/InstCombine/shufflevector-div-rem-inseltpoison.ll

[PATCH] D93817: Update transformations to use poison for insertelement/shufflevector's placeholder value

2020-12-31 Thread Juneyoung Lee via Phabricator via cfe-commits
aqjune updated this revision to Diff 314175. aqjune added a comment. Find a few missing places & update them to use poison Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93817/new/ https://reviews.llvm.org/D93817 Files: clang/test/CodeGen/SystemZ

[PATCH] D93817: Update transformations to use poison for insertelement/shufflevector's placeholder value

2020-12-30 Thread Juneyoung Lee via Phabricator via cfe-commits
aqjune added a comment. I'll share the result after checking the validity of this patch using alive2. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93817/new/ https://reviews.llvm.org/D93817 ___ cfe-comm

[PATCH] D93817: Update transformations to use poison for new insertelement's placeholder value

2020-12-30 Thread Juneyoung Lee via Phabricator via cfe-commits
aqjune updated this revision to Diff 314123. aqjune added a comment. Herald added subscribers: cfe-commits, pengfei, aheejin, jgravelle-google, sbc100, dschuff. Herald added a project: clang. Rebase, update transformations with shufflevector involved too Repository: rG LLVM Github Monorepo C

[PATCH] D93923: Use unary CreateShuffleVector if possible

2020-12-30 Thread Juneyoung Lee via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG9b29610228c8: Use unary CreateShuffleVector if possible (authored by aqjune). Changed prior to commit: https://reviews.llvm.org/D93923?vs=314108&i

[PATCH] D93923: Use unary CreateShuffleVector if possible

2020-12-30 Thread Juneyoung Lee via Phabricator via cfe-commits
aqjune updated this revision to Diff 314108. aqjune added a comment. - clang-format, address warnings Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93923/new/ https://reviews.llvm.org/D93923 Files: clang/lib/CodeGen/CGBuiltin.cpp clang/lib/Cod

[PATCH] D93923: Use unary CreateShuffleVector if possible

2020-12-30 Thread Juneyoung Lee via Phabricator via cfe-commits
aqjune added a comment. Thanks! :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93923/new/ https://reviews.llvm.org/D93923 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/

[PATCH] D93923: Use unary CreateShuffleVector if possible

2020-12-29 Thread Juneyoung Lee via Phabricator via cfe-commits
aqjune added inline comments. Comment at: llvm/lib/Transforms/Scalar/SROA.cpp:2260 - V = IRB.CreateSelect(ConstantVector::get(Mask), V, Old, Name + "blend"); + V = IRB.CreateSelect(ConstantVector::get(Mask2), V, Old, Name + "blend"); LLVM_DEBUG(dbgs() << "blend: " << *

[PATCH] D93923: Use unary CreateShuffleVector if possible

2020-12-29 Thread Juneyoung Lee via Phabricator via cfe-commits
aqjune created this revision. aqjune added reviewers: spatel, nikic, lebedev.ri, craig.topper, RKSimon, efriedma. Herald added subscribers: dexonsmith, kerbowa, pengfei, dmgreen, hiraditya, nhaehnle, jvesely, nemanjai, arsenm. aqjune requested review of this revision. Herald added projects: clang

[PATCH] D93793: [IR] Let IRBuilder's CreateVectorSplat/CreateShuffleVector use poison as placeholder

2020-12-29 Thread Juneyoung Lee via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. aqjune marked an inline comment as done. Closed by commit rG278aa65cc495: [IR] Let IRBuilder's CreateVectorSplat/CreateShuffleVector use poison as… (authored by aqjune)

[PATCH] D93793: [IR] Let IRBuilder's CreateVectorSplat/CreateShuffleVector use poison as placeholder

2020-12-29 Thread Juneyoung Lee via Phabricator via cfe-commits
aqjune marked an inline comment as done. aqjune added inline comments. Comment at: llvm/lib/IR/IRBuilder.cpp:1020 Value *Zeros = ConstantAggregateZero::get(VectorType::get(I32Ty, EC)); - return CreateShuffleVector(V, Undef, Zeros, Name + ".splat"); + return CreateShuffleVect

[PATCH] D93793: [IR] Let IRBuilder's CreateVectorSplat/CreateShuffleVector use poison as placeholder

2020-12-29 Thread Juneyoung Lee via Phabricator via cfe-commits
aqjune updated this revision to Diff 313989. aqjune added a comment. Herald added a reviewer: bollu. Update comments Call unary CreateShuffleVector Update polly tests Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93793/new/ https://reviews.llvm.org

[PATCH] D93793: [IR] Let IRBuilder's CreateVectorSplat/CreateShuffleVector use poison as placeholder

2020-12-29 Thread Juneyoung Lee via Phabricator via cfe-commits
aqjune added inline comments. Comment at: clang/test/CodeGen/SystemZ/builtins-systemz-zvector-constrained.c:84 vd = vec_splat(vd, 1); // CHECK: shufflevector <2 x double> %{{.*}}, <2 x double> undef, <2 x i32> // CHECK-ASM: vrepg These SystemZ zvector

[PATCH] D93793: [IR] Let IRBuilder's CreateVectorSplat use poison as inselt's placeholder

2020-12-29 Thread Juneyoung Lee via Phabricator via cfe-commits
aqjune updated this revision to Diff 313945. aqjune added a comment. Herald added subscribers: kerbowa, nhaehnle, jvesely. Update IRBuilder to fill poison at shufflevector's empty operand Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93793/new/ htt

[PATCH] D93793: [IR] Let IRBuilder's CreateVectorSplat use poison as inselt's placeholder

2020-12-27 Thread Juneyoung Lee via Phabricator via cfe-commits
aqjune added inline comments. Comment at: llvm/lib/IR/IRBuilder.cpp:1021 Value *Zeros = ConstantAggregateZero::get(VectorType::get(I32Ty, EC)); return CreateShuffleVector(V, Undef, Zeros, Name + ".splat"); } aqjune wrote: > nikic wrote: > > aqjune wrote: >

[PATCH] D93586: [InstCombine] use poison as placeholder for undemanded elems

2020-12-27 Thread Juneyoung Lee via Phabricator via cfe-commits
aqjune added a comment. Okay, I'll gently land this. I was just wondering whether insertelements having heterogenous placeholder would be problematic (this patch makes some of insertelement use poison, but not all), but it may not matter very much. Repository: rG LLVM Github Monorepo CHANGE

[PATCH] D93793: [IR] Let IRBuilder's CreateVectorSplat use poison as inselt's placeholder

2020-12-27 Thread Juneyoung Lee via Phabricator via cfe-commits
aqjune added inline comments. Comment at: llvm/lib/IR/IRBuilder.cpp:1021 Value *Zeros = ConstantAggregateZero::get(VectorType::get(I32Ty, EC)); return CreateShuffleVector(V, Undef, Zeros, Name + ".splat"); } nikic wrote: > aqjune wrote: > > nlopes wrote: >

[PATCH] D93586: [InstCombine] use poison as placeholder for undemanded elems

2020-12-27 Thread Juneyoung Lee via Phabricator via cfe-commits
aqjune added a comment. There are 3 more patches: https://reviews.llvm.org/D93793 (IRBuilder's CreateVectorSplat) https://reviews.llvm.org/D93817 (Other transformations) https://reviews.llvm.org/D93818 (LangRef) Would it be desirable if I land all of these at once as well as this (93586) when

[PATCH] D93586: [InstCombine] use poison as placeholder for undemanded elems

2020-12-25 Thread Juneyoung Lee via Phabricator via cfe-commits
aqjune added a comment. I'll make two more patches - the instsimplify/vectorizer/ changes that make insertelement poison, and the langref update to shufflevector. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93586/new/ https://reviews.llvm.or

[PATCH] D93793: [IR] Let IRBuilder's CreateVectorSplat use poison as inselt's placeholder

2020-12-25 Thread Juneyoung Lee via Phabricator via cfe-commits
aqjune added inline comments. Comment at: llvm/lib/IR/IRBuilder.cpp:1021 Value *Zeros = ConstantAggregateZero::get(VectorType::get(I32Ty, EC)); return CreateShuffleVector(V, Undef, Zeros, Name + ".splat"); } nlopes wrote: > while at it, don't you want to c

[PATCH] D93793: [IR] Let IRBuilder's CreateVectorSplat use poison as inselt's placeholder

2020-12-23 Thread Juneyoung Lee via Phabricator via cfe-commits
aqjune created this revision. aqjune added reviewers: spatel, lebedev.ri, efriedma, nlopes, regehr, RKSimon, zhengyangl, nikic, hfinkel. Herald added subscribers: dmgreen, kbarton, hiraditya, nemanjai. aqjune requested review of this revision. Herald added projects: clang, LLVM. Herald added subsc

[PATCH] D93586: [InstCombine] use poison as placeholder for undemanded elems

2020-12-23 Thread Juneyoung Lee via Phabricator via cfe-commits
aqjune added a comment. In D93586#2468350 , @spatel wrote: > It would be good to update those for consistency; the codegen tests are > supposed to be representative of what comes out of the IR optimizer. IIUC, we > could do the substitution on those file

[PATCH] D93586: [InstCombine] use poison as placeholder for undemanded elems

2020-12-22 Thread Juneyoung Lee via Phabricator via cfe-commits
aqjune added a comment. In D93586#2467844 , @RKSimon wrote: > I'm sorry I've only just started looking at this - are you saying that you > want to handle all "insertelement undef" cases in one go and not just a > series of patcches after this one? It wi

[PATCH] D93586: [InstCombine] use poison as placeholder for undemanded elems

2020-12-21 Thread Juneyoung Lee via Phabricator via cfe-commits
aqjune added a comment. > There are 792 files in llvm/test, most of which are in test/Transform (119) > and test/CodeGen(655). > The transition will be swiftly done (if there's no other issue hopefully) by > the next weekend. Thinking about these again, do we need to make a poison copy for test

[PATCH] D93586: [InstCombine] use poison as placeholder for undemanded elems

2020-12-21 Thread Juneyoung Lee via Phabricator via cfe-commits
aqjune added a comment. In D93586#2466284 , @aqjune wrote: > The transition will be swiftly done (if there's no other issue hopefully) by > the next weekend. Oops, I meant this weekend hopefully. Repository: rG LLVM Github Monorepo CHANGES SINCE LAS

[PATCH] D93586: [InstCombine] use poison as placeholder for undemanded elems

2020-12-21 Thread Juneyoung Lee via Phabricator via cfe-commits
aqjune added a comment. In D93586#2464841 , @spatel wrote: > Besides changing IRBuilder and shufflevector's definition, I think we'll also > need updates in the vectorizers, InstSimplify, and other places in > InstCombine that use UndefValue for InsertEl

[PATCH] D85788: [Clang test] Update to allow passing extra default clang arguments in use_clang

2020-12-11 Thread Juneyoung Lee via Phabricator via cfe-commits
aqjune added a comment. In D85788#2444136 , @guiand wrote: > IMO it's better to just one-and-done programatically add `-Xclang > -disable-noundef-analysis` to all the tests' RUN lines. That way there are no > hidden flags. If a script for this can be wr

[PATCH] D81678: Introduce noundef attribute at call sites for stricter poison analysis

2020-12-08 Thread Juneyoung Lee via Phabricator via cfe-commits
aqjune added a comment. In D81678#2438264 , @eugenis wrote: > This change looks fine to me, but I'm slightly concerned about > https://reviews.llvm.org/D85788 - see my last comment on that revision. Thank you for the link! I left a comment there. Repos

[PATCH] D85788: [Clang test] Update to allow passing extra default clang arguments in use_clang

2020-12-08 Thread Juneyoung Lee via Phabricator via cfe-commits
aqjune added a comment. Herald added a subscriber: frasercrmck. In D85788#2335838 , @eugenis wrote: > I wonder it can be avoided by > > - disable noundef analysis by default in cc1 > - always add -enable-noundef-analysis in the driver when invoking cc1 Wo

[PATCH] D81678: Introduce noundef attribute at call sites for stricter poison analysis

2020-12-07 Thread Juneyoung Lee via Phabricator via cfe-commits
aqjune added a comment. Hello all, What is the status of this patch? Do we need someone who look into the lit change? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D81678/new/ https://reviews.llvm.org/D81678

[PATCH] D92270: [ConstantFold] Fold more operations to poison

2020-11-30 Thread Juneyoung Lee via Phabricator via cfe-commits
aqjune added a comment. In D92270#2422661 , @uabelho wrote: > Should langref also be updated to describe this? Or does it already? > I just found this > "An instruction that depends on a poison value, produces a poison value > itself." > here > http://l

[PATCH] D92270: [ConstantFold] Fold more operations to poison

2020-11-30 Thread Juneyoung Lee via Phabricator via cfe-commits
aqjune added a comment. Hi, It seems it is related to two optimizations: (1) `select i1 x, true, y` -> `or i1 x, y` (2) `or i1 x, poison` -> `poison` Semantically, the first one is broken. It needs to freeze y. But, it will introduce a lot of freeze instructions. The clang patches that introduc

[PATCH] D92270: [ConstantFold] Fold more operations to poison

2020-11-29 Thread Juneyoung Lee via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG53040a968dc2: [ConstantFold] Fold more operations to poison (authored by aqjune). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

  1   2   >