Page MenuHomePhabricator

hsaito (Hideki Saito)
User

Projects

User does not belong to any projects.

User Details

User Since
Jul 25 2016, 9:02 AM (168 w, 2 d)

Recent Activity

Today

hsaito added inline comments to D67948: [LV] Interleaving should not exceed estimated loop trip count..
Wed, Oct 16, 10:18 AM · Restricted Project
hsaito added a comment to D67948: [LV] Interleaving should not exceed estimated loop trip count..

@hsaito, Please commit if you find this version acceptable.

Wed, Oct 16, 10:10 AM · Restricted Project

Mon, Oct 14

hsaito added a comment to D68814: [LV] Allow assume calls in predicated blocks..

If conditional assumes are to be dropped, better do so on entry to VPlan, as in DeadInstructions, rather than representing them in ReplicateRecipe (as do unconditional assumes) and silencing their code generation.

To retain conditional assumes along with their control flow, they could be marked under isScalarWithPredication; but this complicates vectorization, plus what use are such assumes when all else is if-converted(?)

Conditional assumes under uniform control flow could be retained, along with the uniform control flow they depend upon; this may be mostly relevant for outerloop vectorization.

Mon, Oct 14, 10:52 AM · Restricted Project

Fri, Oct 11

hsaito added a comment to D68651: [InstCombine] Signed saturation patterns.

I'm in favor of treating signed saturation as canonical. The issue in delaying detection of such cases to instruction selection is the volatility of the IR: there is no guarantee that the IR will remain in the same form (expected by isel) from one day to the next. For example, some optimization may decide to just promote the operations to the wider type and only do the extension/truncate once, depending on how many saturating operations may be near one another. Handling this variability in isel is just not feasible.

Fri, Oct 11, 5:00 PM · Restricted Project

Thu, Oct 10

hsaito accepted D66199: [docs] loop pragmas.

LGTM. Please wait for a few days in case others have more comments.

Thu, Oct 10, 11:22 AM · Restricted Project
hsaito added inline comments to D66199: [docs] loop pragmas.
Thu, Oct 10, 9:54 AM · Restricted Project

Wed, Oct 9

hsaito added a comment to D68651: [InstCombine] Signed saturation patterns.

I haven't looked at the patch in detail, but as author of at least part of the prior art cited here, I agree with the direction*. I also participated in some of the vector idioms discussions from a few years ago. There's overlap with the vector idiom problems, but as noted, these are generic (scalar too) math ops, so it's not exactly the same. We invested significantly in IR analysis and codegen for the math intrinsics, so that may have changed the thinking. I don't remember the sequence of events or if there was a dedicated llvm-dev thread for this, but the general idea is that if we have a generic intrinsic for the math and can easily invert the transform in the backend for targets/types that are not supported, try to canonicalize to the intrinsic.

Wed, Oct 9, 11:08 AM · Restricted Project

Tue, Oct 8

hsaito added a comment to D68651: [InstCombine] Signed saturation patterns.

We do form uadd_sat as in rL357012 and usub_sat from selects.

I really just need some way to generate sadd_sats for vectorisation. If there's a better way than this, I'm all ears :)

Tue, Oct 8, 2:33 PM · Restricted Project

Mon, Oct 7

hsaito accepted D67948: [LV] Interleaving should not exceed estimated loop trip count..

Vectorizer code change looks fine with me. I'd like to see the comments updated, though. Any more changes needed for the LIT tests?

Mon, Oct 7, 11:32 AM · Restricted Project

Fri, Oct 4

hsaito added inline comments to D67948: [LV] Interleaving should not exceed estimated loop trip count..
Fri, Oct 4, 4:40 PM · Restricted Project
hsaito added a comment to D68082: [LV] Emitting SCEV checks with OptForSize.
>> In this regard, I do not like pass managers running Analyses for a Transformation pass w/o first letting the Transformation pass inspect the incoming IR, but that's a totally different discussion and I don't have a solution for that problem.

Maybe I'm misunderstanding, but SCEV is lazy so if a transform looks at the IR and decides not to ask SCEV for trip count or call getSCEV then SCEV will not do any actual work.

Fri, Oct 4, 3:44 PM · Restricted Project
hsaito added a comment to D67948: [LV] Interleaving should not exceed estimated loop trip count..

Vectorizer code change looks fine with me. I'd like to see the comments updated, though. Any more changes needed for the LIT tests?

Fri, Oct 4, 2:21 PM · Restricted Project
hsaito added a comment to D68082: [LV] Emitting SCEV checks with OptForSize.

I agree with @Ayal about changing getAsAddRec for OptForSize. In general, it's better to not "Analyze" if we know we won't be using the result of analysis.

Fri, Oct 4, 12:35 PM · Restricted Project

Wed, Sep 18

hsaito accepted D67690: [LV][NFC] Factor out calculation of "best" estimated trip count..

LGTM

Wed, Sep 18, 12:50 PM · Restricted Project

Tue, Sep 17

hsaito accepted D67690: [LV][NFC] Factor out calculation of "best" estimated trip count..

Nice clean up of the code as well. LGTM.

Tue, Sep 17, 11:18 PM · Restricted Project

Sep 9 2019

hsaito added a comment to D66796: [clang] Loop pragma vectorize(disable).

That's exactly the reason why I think vectorize(disable) should disable vectorisation for that loop. I just don't see what else a user would expect.

Sep 9 2019, 5:38 PM
hsaito added a comment to D66796: [clang] Loop pragma vectorize(disable).

There are two ways to think.

  1. vectorize(disable) as in disable the LoopVectorize pass itself.
  2. vectorize(disable) as in disabling the loop vectorization transformation
Sep 9 2019, 12:56 PM

Aug 6 2019

hsaito added a comment to D62997: [LV] Share the LV illegality reporting with LoopVectorize. NFC..

Nope, Asan failures from the sanitizer commit.

Aug 6 2019, 9:43 AM · Restricted Project
hsaito added a comment to D62997: [LV] Share the LV illegality reporting with LoopVectorize. NFC..

Buildbot failures reported so far. Just FYI. I don't think any actions from this commit is needed.

Aug 6 2019, 9:33 AM · Restricted Project

Aug 5 2019

hsaito committed rGec818d7fb3c4: [LV][NFC] Share the LV illegality reporting with LoopVectorize. (authored by hsaito).
[LV][NFC] Share the LV illegality reporting with LoopVectorize.
Aug 5 2019, 11:09 PM
hsaito added a comment to D62997: [LV] Share the LV illegality reporting with LoopVectorize. NFC..

@rengolin @hsaito I understand you were busy during the version 9 pre-releasing. But if you have time and the diff looks good for you, could you land it? Thank you.

I can land this today.

Aug 5 2019, 11:08 PM · Restricted Project
hsaito committed rL367980: [LV][NFC] Share the LV illegality reporting with LoopVectorize..
[LV][NFC] Share the LV illegality reporting with LoopVectorize.
Aug 5 2019, 11:08 PM
hsaito closed D62997: [LV] Share the LV illegality reporting with LoopVectorize. NFC..
Aug 5 2019, 11:08 PM · Restricted Project
hsaito added a comment to D62997: [LV] Share the LV illegality reporting with LoopVectorize. NFC..

@rengolin @hsaito I understand you were busy during the version 9 pre-releasing. But if you have time and the diff looks good for you, could you land it? Thank you.

Aug 5 2019, 1:33 AM · Restricted Project

Aug 2 2019

hsaito added a comment to D63981: [LV] Avoid building interleaved group in presence of WAW dependency.

Reported buildbot fails for Evgueni to follow up as needed:

Aug 2 2019, 4:19 PM · Restricted Project
hsaito added a comment to D63981: [LV] Avoid building interleaved group in presence of WAW dependency.

LIT test has been moved to X86 subdir, in response to ps4-buildslave1 buildbot failure.

Aug 2 2019, 12:26 AM · Restricted Project
hsaito committed rG8871ac41a728: Moves the newly added test interleaved-accesses-waw-dependency.ll to X86… (authored by hsaito).
Moves the newly added test interleaved-accesses-waw-dependency.ll to X86…
Aug 2 2019, 12:25 AM
hsaito committed rL367659.
Aug 2 2019, 12:25 AM

Aug 1 2019

hsaito added a comment to D63981: [LV] Avoid building interleaved group in presence of WAW dependency.

r367654 | hsaito | 2019-08-01 23:31:50 -0700 (Thu, 01 Aug 2019) | 12 lines

Aug 1 2019, 11:41 PM · Restricted Project
hsaito committed rG09fac2450b19: [LV] Avoid building interleaved group in presence of WAW dependency (authored by hsaito).
[LV] Avoid building interleaved group in presence of WAW dependency
Aug 1 2019, 11:36 PM
hsaito committed rL367654.
Aug 1 2019, 11:31 PM
hsaito closed D63981: [LV] Avoid building interleaved group in presence of WAW dependency.
Aug 1 2019, 11:31 PM · Restricted Project
hsaito added a comment to D63981: [LV] Avoid building interleaved group in presence of WAW dependency.

Thanks for quick response @hsaito ! May I ask you to commit it as well since I don't have committer rights yet.

Aug 1 2019, 5:29 PM · Restricted Project

Jul 31 2019

hsaito accepted D63981: [LV] Avoid building interleaved group in presence of WAW dependency.

Indeed, this is a much cleaner fix. LGTM.

Jul 31 2019, 10:14 AM · Restricted Project
hsaito added a comment to D63981: [LV] Avoid building interleaved group in presence of WAW dependency.

Hi @hsaito!

Would be nice if you find time to take a look.

Jul 31 2019, 9:56 AM · Restricted Project
hsaito accepted D65197: [LV] Tail-loop Folding.

LGTM, pending the discussion about the exact meaning of the newly introduced "vector predicate" pragma (expect this to happen outside of this review). Please wait for another day to give others last minute opportunity to give feedback.

Jul 31 2019, 9:54 AM · Restricted Project

Jul 29 2019

hsaito added a comment to D65197: [LV] Tail-loop Folding.

Friendly ping :-)

Jul 29 2019, 12:53 PM · Restricted Project

Jul 24 2019

hsaito added a comment to D65197: [LV] Tail-loop Folding.

We probably need to discuss whether vectorize_predicate(enable) should (or should not) implicitly turns on vectorize(enable) or not. I guess the current behavior is "does not", right? We don't have to discuss that in this review, but we still want to make a conscious decision one way or the other, or did I miss that discussion?

Jul 24 2019, 12:42 PM · Restricted Project
hsaito accepted D64916: [LV] Scalar Epilogue Lowering. NFC..

LGTM

Jul 24 2019, 12:12 PM · Restricted Project
hsaito added a comment to D64916: [LV] Scalar Epilogue Lowering. NFC..

Just one comment from me.

Jul 24 2019, 10:14 AM · Restricted Project

Jul 19 2019

hsaito added a comment to D64916: [LV] Scalar Epilogue Lowering. NFC..

Looks to be a good change. Can we add a little more improvement to this patch --- adding more crispness in ORE message?

Jul 19 2019, 9:24 AM · Restricted Project

Jul 18 2019

hsaito added a comment to D62997: [LV] Share the LV illegality reporting with LoopVectorize. NFC..

Aha, that was a "hacky" way to get "loop contains a switch statement" along with the warning. I see. I suppose we can't blindly use LV_NAME, then.
I then suppose some tests (like pr38800.ll) didn't even need -pass-remarks-missed flag (which is incorrectly used, I think).

Jul 18 2019, 11:43 AM · Restricted Project

Jul 17 2019

hsaito added a comment to D62997: [LV] Share the LV illegality reporting with LoopVectorize. NFC..

The test no_switch.ll has been updated: the LV doesn't report remarks without -pass-remarks-missed='loop-vectorize' -pass-remarks-analysis='loop-vectorize', so the test from the CHECK section is removed. I have no idea why, but remarks with lambdas through the code emitted via OptimizationRemarkMissed, not OptimizationRemarkAnalysis (if I right understand, 'Missed' means the pass has not been applied by any reasons during optimization).

Jul 17 2019, 10:58 AM · Restricted Project

Jun 25 2019

hsaito added a comment to D62997: [LV] Share the LV illegality reporting with LoopVectorize. NFC..

Looking at the expected output and the explanations on -Rpass* flags, it could be that those tests should be using -pass-remarks-analysis=loop-vectorize, instead of -pass-remarks-missed=loop-vectorize. Would you try?

Jun 25 2019, 10:38 AM · Restricted Project

Jun 21 2019

hsaito added a comment to D62997: [LV] Share the LV illegality reporting with LoopVectorize. NFC..

The diff has been updated: private methods were removed, the function 'reportVectorizationFailure' was moved to LoopVectorize.cpp and declared in LoopVectorize.h (namespace llvm). I've removed the passName parameter and pass LV_NAME as pass name to ORE but this change breaks 3 regression tests:

I'm assuming that the pass name isn't just LV_NAME, but whichever pass is using that function, which now that it's higher level, can be anything.

@rengolin @hsaito I need you suggestions how to deal with the Hints.vectorizeAnalysisPassName() method, it is very long to always call it whenever the reportVectorizationFailure is invoked.

I don't have a better idea on the top of my head, but this sounds like a change that can be done later as a quick refactory once someone comes up with a clever replacement.

I'd be ok having that for now... @hsaito may have a better idea, though.

--renato

Jun 21 2019, 12:26 PM · Restricted Project

Jun 14 2019

hsaito added a comment to D62997: [LV] Share the LV illegality reporting with LoopVectorize. NFC..

Sorry, I missed the original review request in the e-mail pile up. Yes, this is the direction I was suggesting. Thank you.

Jun 14 2019, 12:16 PM · Restricted Project

May 30 2019

Vladimir Lazarev <vladimir.lazarev@intel.com> committed rG45b17d6c63f7: [SYCL] add optimized vec class constructors (authored by hsaito).
[SYCL] add optimized vec class constructors
May 30 2019, 8:05 AM
vladimirlaz <vladimir.lazarev@intel.com> committed rG60befd246ea6: [SYCL] Change Indexer to use C++11 feature, from C++14. (authored by hsaito).
[SYCL] Change Indexer to use C++11 feature, from C++14.
May 30 2019, 8:02 AM
vladimirlaz <vladimir.lazarev@intel.com> committed rG8b1894b4f3a6: [SYCL] Improve SYCL vector implementation (authored by hsaito).
[SYCL] Improve SYCL vector implementation
May 30 2019, 8:01 AM

May 28 2019

hsaito added a comment to D62478: [LV] Wrap LV illegality reporting in a function. NFC..

@rengolin It is because the function uses OptimizationRemarkEmitter *ORE, a member of the LoopVectorizationLegality class.

I see. Given the function has a lot of arguments already and it really isn't used elsewhere, I'd rather just add ORE to the args list.

Unless @hsaito or @fhahn think this could be used elsewhere in the vectorizer, then it shouldn't be in that class anyway.

May 28 2019, 10:37 AM · Restricted Project

May 24 2019

hsaito added a comment to D62311: [LV] Inform about exactly reason of loop illegality.

@hsaito I agree, to have a function to report about an error is a good idea.

I'm new in LLVM community, so what does NFC mean? Should I close this review and open a new one or you mean just to upload a new diff for comments?

May 24 2019, 11:08 AM · Restricted Project
hsaito added a comment to D62311: [LV] Inform about exactly reason of loop illegality.

While we are looking at this, I'd like to discuss how to make these things easier. I think there a merit in using a utility function that takes three strings, something along the lines of
the following pseudo code:

May 24 2019, 10:38 AM · Restricted Project

May 9 2019

hsaito added a comment to D32530: [SVE][IR] Scalable Vector IR Type.

What's the status of this? It seems like discussion has died down a bit. I think Graham's idea to change from <scalable 2 x float> to <vscale x 2 x float> will make the IR more readable/understandable but it's not a show-stopper for me.

Are there any other outstanding issues to address before this lands?

May 9 2019, 10:45 AM · Restricted Project

Apr 30 2019

hsaito added inline comments to D61030: [PassManagerBuilder] Add option for interleaved loops, for loop vectorize..
Apr 30 2019, 9:27 AM · Restricted Project

Apr 29 2019

hsaito added a comment to D61030: [PassManagerBuilder] Add option for interleaved loops, for loop vectorize..

I verified that the two unroll flags propagate to the option set in the PassManagerBuilder. This change + the clang change in D61142 should not make any visible change for clang users.

Apr 29 2019, 6:07 PM · Restricted Project
hsaito added reviewers for D61030: [PassManagerBuilder] Add option for interleaved loops, for loop vectorize.: hfinkel, rengolin, mkuper, fhahn, hsaito.
Apr 29 2019, 6:06 PM · Restricted Project

Apr 25 2019

hsaito added a comment to D61030: [PassManagerBuilder] Add option for interleaved loops, for loop vectorize..

You mean, clang setting EnableLoopInterleaving.

Actually I meant DisableUnrollLoops in the PassManagerBuilder. It's currently always set to false, not based on a flag, and I found a single instance where it's value is changed in clang (see clang patch).

I'm happy to make whatever change needed to clang first, but I don't see what that change is. If you could point me to what I've missed, that would be great!

Apr 25 2019, 5:41 PM · Restricted Project
hsaito added a comment to D61030: [PassManagerBuilder] Add option for interleaved loops, for loop vectorize..

Thanks a lot for the suggestion! Sent: http://lists.llvm.org/pipermail/llvm-dev/2019-April/131968.html

My intention was to not make any change visible to clang.
If clang currently sets the DisableUnrollLoops, and llvm will not use that for LoopVectorization, then have clang set LoopsInterleaved to the same value as the one used for unroll loops.

Apr 25 2019, 12:42 PM · Restricted Project
hsaito added a comment to D61030: [PassManagerBuilder] Add option for interleaved loops, for loop vectorize..

Update test.

Apr 25 2019, 11:30 AM · Restricted Project

Apr 24 2019

hsaito added a comment to D32530: [SVE][IR] Scalable Vector IR Type.

Exactly. Non-constant values can become constant. Constant values can be guarded by vscale-dependent runtime guards (both hand-written and compiler generated). My preference is to leave this not restricted to vscale == 1 values, but rather allow all values that can be supported at runtime, and have it be UB if, at runtime, the relevant index is not available.

+1

Apr 24 2019, 10:23 AM · Restricted Project

Apr 23 2019

hsaito added a comment to D32530: [SVE][IR] Scalable Vector IR Type.

I am not sure how it could be anything but n. If you don't know how long the vector is, you can't correctly generate an index beyond n.

But you know at runtime... there has to be a way to determine, at runtime, vscale. And the index doesn't need to be a constant. I'm not sure that we need to restrict non-constant n to only values valid for vscale == 1.

Good point. 100% agree. I was only considering the constant case.

Ok, so do we have agreement that constant literal indices should be limited to 0..n-1 for now, but non-constant indices can potentially exceed n so that expressions featuring vscale can be used?

Apr 23 2019, 10:39 AM · Restricted Project

Apr 17 2019

hsaito added a comment to D57504: RFC: Prototype & Roadmap for vector predication in LLVM.

Do we really need both vp.fadd() and vp.constrained.fadd()? Can't we just use the latter with rmInvalid/ebInvalid? That should prevent vp.constrained.fadd from losing optimizations w/o good reasons.

According to the LLVM langref, "fpexcept.ignore" seems to be the right option for exceptions whereas there is no "round.permissive" option for the rounding behavior. Abusing rmInvalid/ebInvalid seems hacky.

Apr 17 2019, 9:54 AM · Restricted Project

Apr 16 2019

hsaito added a comment to D57504: RFC: Prototype & Roadmap for vector predication in LLVM.
    1. Updates
  • added constrained fp intrinsics (IR level only).
  • initial support for mapping llvm.experimental.constrained.* intrinsics to llvm.vp.constrained.*.
Apr 16 2019, 2:27 PM · Restricted Project

Apr 15 2019

hsaito added a comment to D32530: [SVE][IR] Scalable Vector IR Type.

I think this is a coherent set of changes. Given the current top of trunk, this expands support from just assembly/disassembly of machine instructions to include LLVM IR, right? Such being the case, I think this patch should go in. I have some ideas on how to structure passes so SV IR supporting optimizations can be added incrementally. If anyone thinks such a proposal would help, let me know.

I think there is one more thing we still have to do. Does scalable vector type apply to all Instructions where non-scalable vector is allowed? If the answer is no, we need to identify which ones are not allowed to take scalable vector type operand/result. Some of the Instructions are not plain element-wise operation. Do we have agreed upon semantics for all those that are allowed?

The main difference is for 'shufflevector'. For a splat it's simple, since you just use a zeroinitializer mask. For anything else, though, you currently need a constant vector with immediate values; this obviously won't work if you don't know how many elements there are.

Apr 15 2019, 1:04 PM · Restricted Project
hsaito added a comment to D32530: [SVE][IR] Scalable Vector IR Type.

So if we wanted to keep them as intrinsics for now, I think we have one of three options:

  1. Leave discussion on more complicated shuffles until later, and only use scalable autovectorization on loops which don't need anything more than splats.

Given the current state, this is the easiest path.

I agree, although this is an important part of the model, so we should start having this discussion in parallel (sooner rather than later). I had been under the impression that a set of intrinsics were being proposed for this, but extending shufflevector is also an option worth considering. If these are first-class types, then having first-class instruction support is probably the right path. This deserves it's own RFC.

  1. Introduce additional intrinsics for the other shuffle variants as needed
  2. Allow shufflevector to accept arbitrary masks so that intrinsics can be used (though possibly only if the output vector is scalable).

This warrants a larger discussion, which would hinder the current progress.

I agree. We should have a separate RFC on this.

Apr 15 2019, 10:11 AM · Restricted Project

Apr 12 2019

hsaito added a comment to D32530: [SVE][IR] Scalable Vector IR Type.

I think this is a coherent set of changes. Given the current top of trunk, this expands support from just assembly/disassembly of machine instructions to include LLVM IR, right? Such being the case, I think this patch should go in. I have some ideas on how to structure passes so SV IR supporting optimizations can be added incrementally. If anyone thinks such a proposal would help, let me know.

Apr 12 2019, 11:27 AM · Restricted Project

Apr 5 2019

hsaito added inline comments to D32530: [SVE][IR] Scalable Vector IR Type.
Apr 5 2019, 4:02 PM · Restricted Project

Mar 29 2019

hsaito added inline comments to D59723: [NewPassManager] Adding pass tuning options: loop vectorize..
Mar 29 2019, 12:34 PM · Restricted Project

Mar 28 2019

hsaito accepted D59952: [VPLAN] Minor improvement to testing and debug messages..

LGTM

Mar 28 2019, 2:57 PM · Restricted Project
hsaito added inline comments to D59952: [VPLAN] Minor improvement to testing and debug messages..
Mar 28 2019, 12:25 PM · Restricted Project
hsaito added inline comments to D59952: [VPLAN] Minor improvement to testing and debug messages..
Mar 28 2019, 12:19 PM · Restricted Project
hsaito added a comment to D57598: [VPLAN] Determine Vector Width programmatically..

Thanks Francesco!

Mar 28 2019, 11:03 AM · Restricted Project

Mar 27 2019

hsaito added a comment to D57598: [VPLAN] Determine Vector Width programmatically..

Thanks Francesco! I'll commit the change tomorrow, unless @hsaito does it today :)

Mar 27 2019, 2:36 PM · Restricted Project

Mar 25 2019

hsaito added a comment to D57598: [VPLAN] Determine Vector Width programmatically..

@hsaito, I don't have commit access, could you commit this change for me?

Mar 25 2019, 7:58 PM · Restricted Project

Mar 22 2019

hsaito added a comment to D57598: [VPLAN] Determine Vector Width programmatically..

@npanchen , @fhahn , gentle ping :)

Francesco

Mar 22 2019, 3:37 PM · Restricted Project
hsaito added inline comments to D59149: [LV] move useEmulatedMaskMemRefHack() functionality to TTI..
Mar 22 2019, 2:26 PM · Restricted Project

Mar 19 2019

hsaito removed a reviewer for D57978: [CodeGen] Generate follow-up metadata for loops with more than one transformation.: hsaito.
Mar 19 2019, 2:54 PM · Restricted Project, Restricted Project
hsaito added a comment to D57978: [CodeGen] Generate follow-up metadata for loops with more than one transformation..

ping

Mar 19 2019, 2:51 PM · Restricted Project, Restricted Project
hsaito added inline comments to D59149: [LV] move useEmulatedMaskMemRefHack() functionality to TTI..
Mar 19 2019, 10:06 AM · Restricted Project

Mar 14 2019

hsaito accepted D57598: [VPLAN] Determine Vector Width programmatically..

LGTM. Please wait for a few days to give others a chance to go over your updated patch.

Mar 14 2019, 12:21 PM · Restricted Project
hsaito added inline comments to D57598: [VPLAN] Determine Vector Width programmatically..
Mar 14 2019, 10:26 AM · Restricted Project
hsaito added inline comments to D57598: [VPLAN] Determine Vector Width programmatically..
Mar 14 2019, 9:54 AM · Restricted Project

Mar 13 2019

hsaito added a comment to D57598: [VPLAN] Determine Vector Width programmatically..

I mostly changes to code to use the infrastructure that LLVM already provides to determine the vectorization factor.

Mar 13 2019, 1:48 PM · Restricted Project
hsaito added a comment to D59149: [LV] move useEmulatedMaskMemRefHack() functionality to TTI..

Any improvement is an improvement so I am happy with that but it is still mentioned that this solution is a hack and I guess the

Cost model for emulated masked load/store is completely broken.

comment is still valid. What would it take to address this properly?

Mar 13 2019, 12:43 PM · Restricted Project

Mar 8 2019

hsaito added inline comments to D59149: [LV] move useEmulatedMaskMemRefHack() functionality to TTI..
Mar 8 2019, 12:16 PM · Restricted Project
hsaito created D59149: [LV] move useEmulatedMaskMemRefHack() functionality to TTI..
Mar 8 2019, 12:14 PM · Restricted Project

Feb 6 2019

hsaito added a comment to D57837: [LV] Prevent interleaving if computeMaxVF returned None..

This LGTM with the minor documentation comments and retention of UserIC.
Seems like we've reached a consensus, correct me if not.
Thanks!

Feb 6 2019, 4:18 PM · Restricted Project
hsaito added a comment to D57837: [LV] Prevent interleaving if computeMaxVF returned None..

Agreed, having it done implicitly in a function called computeMaxVF does not seem ideal. From the current behavior of computeMaxVF, I think what we actually want to decide whether to disable interleaving up front is just if we optimize for size or not. What do you think?

Feb 6 2019, 3:14 PM · Restricted Project
hsaito added a comment to D57837: [LV] Prevent interleaving if computeMaxVF returned None..

I have a little mental barrier in accepting this change as is. I think this feeling of mine is mainly due to the name and the "stated" functionality of computeMaxVF and "indirect inference" towards using it for suppressing interleaving. Maybe I'm just being too picky here. If so, my apologies ahead of time.

Feb 6 2019, 2:02 PM · Restricted Project

Feb 5 2019

hsaito added inline comments to D57382: [LV] Move interleave count computation to LVP::plan()..
Feb 5 2019, 4:00 PM · Restricted Project
hsaito added a comment to D57382: [LV] Move interleave count computation to LVP::plan()..

As it is at the moment, we need to know the selected vectorization factor to compute the interleave count. And we need to know the interleave count to know if we need to generate VPlans. Maybe with this refactor it would also make sense to only generate a VPlan for the selected vectorization factor for now? There is no need to build VPlans for multiple vectorization factors in the legacy planning and we have the VPlan native path which builds the VPlans up front for planning.

Feb 5 2019, 3:48 PM · Restricted Project

Feb 1 2019

hsaito added a comment to D57598: [VPLAN] Determine Vector Width programmatically..

While we are at this, let's talk about downstream dependency, if any, for allowing more than one candidate VF along the native path. At least we can write down a list of TODOs so that we'll be aware of the things we need to improve at a time in the future.

Feb 1 2019, 12:39 PM · Restricted Project
hsaito added a reviewer for D57598: [VPLAN] Determine Vector Width programmatically.: npanchen.
Feb 1 2019, 10:00 AM · Restricted Project
hsaito added a comment to D57180: [LV] Avoid adding into interleaved group in presence of WAW dependency.

Rather than over aggressive or too conservative, 1) seems to match the current behavior which forbids store groups with gaps; extending in the other direction will also break the vector WAW dependence, right?

Feb 1 2019, 9:48 AM · Restricted Project
hsaito added a comment to D57180: [LV] Avoid adding into interleaved group in presence of WAW dependency.

(I am saying that after recently investigating a few cases of hour-long compile times on user code that were caused by unnecessary scanning)

Feb 1 2019, 9:45 AM · Restricted Project

Jan 31 2019

hsaito added a comment to D57180: [LV] Avoid adding into interleaved group in presence of WAW dependency.

I plan on having a look later this week. I am a little worried that the checks in-line here are already quite complex and I would like to have a think if that could be improved in some way.

I agree; The algorithm makes sure that we visit everything between B and A, including C, before we visit A; so we have a chance to identify the (potentially) interfering store C before we reach A; This is what allows the algorithm to only compare the pairs (A,B) without having each time to also scan everything in between.

So I think the bug is that when we visited C, and found that it could be inserted into B's group dependence-wise, but wasn't inserted due to other reasons, we should have either:

  1. Invalidated the group (which is over aggressive but better than wrong code)
  2. Recorded in B's Group the index where C could be inserted, to "burn" that index from allowing some other instruction A to become a group member at that index; so when we reach A we see its spot is taken. (I think this will have the same effect as the proposed patch but without the extra scan.)
  3. Same as above but instead of bailing out on grouping A with B, make sure that C is also sunk down with that group (as I think Hideki mentioned in the PR) (maybe a future improvement).
Jan 31 2019, 11:42 PM · Restricted Project

Jan 23 2019

hsaito committed rL351990.
Jan 23 2019, 2:43 PM
hsaito closed D53349: [VPlan] Changes to implement VPlan based predication for VPlan-native path..
Jan 23 2019, 2:43 PM

Dec 19 2018

hsaito added a comment to D53865: [LoopVectorizer] Improve computation of scalarization overhead..

Unfortunately, this could not be done with computeInstDiscount() as a simple extension of (1), since memory accesses are dealt with before anything else.

Dec 19 2018, 3:23 PM

Dec 12 2018

hsaito added a comment to D53865: [LoopVectorizer] Improve computation of scalarization overhead..

I think you are either 1) arm-twisting the vectorizer to emit vector code which you know will be scalar or 2) arm-twisting vectorizer's cost model to believe what you are emitting as "vector" to be really scalar. I certainly do not see the reason why "you have to" do that, because letting vectorizer emit scalar IR instructions in those cases should be "equivalent". So, why "do you WANT to" do that? IR going out of vectorizer may be more compact, but what that'll accomplish is cheating all downstream optimizers and their cost models.

I am just trying to keep it simple by not changing how LV generates code, but merely improve the cost computations. Changing the output of a vectorized loop seems like a much bigger project, which I did not attempt.

Dec 12 2018, 1:27 PM