Page MenuHomePhabricator

masoud.ataei (Masoud Ataei)
User

Projects

User does not belong to any projects.

User Details

User Since
Nov 9 2018, 8:48 AM (106 w, 5 d)

Recent Activity

Yesterday

masoud.ataei committed rGb86a1cd2f854: [PowerPC] dyn_cast should be dyn_cast_or_null in MASSV pass (authored by masoud.ataei).
[PowerPC] dyn_cast should be dyn_cast_or_null in MASSV pass
Tue, Nov 24, 8:22 AM
masoud.ataei closed D91729: [PowerPC] dyn_cast should be dyn_cast_or_null in MASSV pass.
Tue, Nov 24, 8:22 AM · Restricted Project

Thu, Nov 19

masoud.ataei added inline comments to D91729: [PowerPC] dyn_cast should be dyn_cast_or_null in MASSV pass.
Thu, Nov 19, 10:14 AM · Restricted Project

Wed, Nov 18

masoud.ataei requested review of D91729: [PowerPC] dyn_cast should be dyn_cast_or_null in MASSV pass.
Wed, Nov 18, 11:22 AM · Restricted Project

Jun 12 2020

masoud.ataei committed rG2d038370bb6b: DAGCombiner optimization for pow(x,0.75) and pow(x,0.25) on double and single… (authored by masoud.ataei).
DAGCombiner optimization for pow(x,0.75) and pow(x,0.25) on double and single…
Jun 12 2020, 7:34 AM
masoud.ataei closed D80744: DAGCombiner optimization for pow(x,0.75) and pow(x,0.25) on double and single precision even in case massv function is asked.
Jun 12 2020, 7:33 AM · Restricted Project, Restricted Project

Jun 10 2020

masoud.ataei updated the diff for D80744: DAGCombiner optimization for pow(x,0.75) and pow(x,0.25) on double and single precision even in case massv function is asked.

Sorry, I forgot the clang-format.

Jun 10 2020, 12:46 PM · Restricted Project, Restricted Project
masoud.ataei retitled D80744: DAGCombiner optimization for pow(x,0.75) and pow(x,0.25) on double and single precision even in case massv function is asked from DAGCombiner optimization for pow(x,0.75) even in case massv function is asked to DAGCombiner optimization for pow(x,0.75) and pow(x,0.25) on double and single precision even in case massv function is asked.
Jun 10 2020, 12:46 PM · Restricted Project, Restricted Project
masoud.ataei updated the diff for D80744: DAGCombiner optimization for pow(x,0.75) and pow(x,0.25) on double and single precision even in case massv function is asked.

I added support for the case when exponent is 0.25 in addition to support for double precision cases.

Jun 10 2020, 12:14 PM · Restricted Project, Restricted Project

Jun 9 2020

masoud.ataei updated the diff for D80744: DAGCombiner optimization for pow(x,0.75) and pow(x,0.25) on double and single precision even in case massv function is asked.

Moving completely the changes from PPCISelLowring.cpp to PPCLowerMASSVEntries.cpp (MASSV pass) to address the reviewer comments.

Jun 9 2020, 5:59 AM · Restricted Project, Restricted Project

Jun 4 2020

masoud.ataei updated the diff for D80744: DAGCombiner optimization for pow(x,0.75) and pow(x,0.25) on double and single precision even in case massv function is asked.

@steven.zhang
I think I had done a terrible mistake. When I tested with my c code, I didn't have if (ClVectorLibrary == TargetLibraryInfoImpl::MASSV) on

if (ClVectorLibrary == TargetLibraryInfoImpl::MASSV)
  setOperationAction(ISD::FPOW, MVT::v4f32, Custom);

and for some reason if I move this check inside the function LowerFPOWMASSV it works good. So I am updating the patch. Thank you for catching it.

Jun 4 2020, 11:33 AM · Restricted Project, Restricted Project

Jun 3 2020

masoud.ataei added a comment to D80744: DAGCombiner optimization for pow(x,0.75) and pow(x,0.25) on double and single precision even in case massv function is asked.

What we are doing is as follows:
llvm.pow(IR) --> FPOW(ISD) --> __powf4_P8/9(ISD/IR)

It makes more sense to do it in the IR pass from what I see. And then, you can query the TargetLibraryInfoImpl::isFunctionVectorizable(name) instead of the option. Maybe, we can do it in ppc pass: PPCLowerMASSVEntries which did something like:

__sind2_massv --> __sind2_P9 for a Power9 subtarget.

Does it make sense ?

I agree in general it makes more sense to do this kind of conversions in an IR pass like PPCLowerMASSVEntries. This is what we are currently doing in LLVM but there is a problem here. If we change the llvm intrinsic to libcall earlier than a later optimization (like in DAGCombiner) the later optimization won't be triggered. In the case that I am proposing the change, if we have pow(x,0.75) in the code, the PPCLowerMASSVEntries pass will currently change it to __powf4_P* libcall then later in the DAGCombiner we will not get the optimization pow(x,0.75) --> sqrt(x)*sqrt(sqrt(x)). So that's a problem, because sqrt(x)*sqrt(sqrt(x)) is faster.

What I am proposing is to move the conversion for powf4 to late in the compiler pipeline. With this change we will we will get above optimization when exponent is 0.75 and MASSV calls otherwise.

If we expect the llvm.pow(x, 0.75) to be lowered as two sqrt and for others, they are libcall, can we just don't transform it as libcall for 0.75 in PPCLowerMASSVEntries? And it will be lowered as two sqrts in DAGCombine.

For pow to be __powf4_P8, there are two step:

  1. in LoopVectorizePass, pow becomes __powf4_massv, then
  2. in PPCLowerMASSVEntries __powf4_massv becomes __powf4_P8

So when we reach PPCLowerMASSVEntries the pow intrinsic is already a libcall. I thought it is really ugly to undo the LoopVectorizePass conversion in PPCLowerMASSVEntries pass when there is an special case.

As what I see from your test is that, we are trying to lower the pow intrinsic(not pow libcall) to two sqrts or __powf4_P8/9. It is different from the case that, pow -> __powf4_massv -> __powf4_P8 as they are all libcall path.

If your intention is to have __powf4_massv transformed to two fsqrts if argument is 0.75, the test is missing in this patch and I am not sure if this patch could work. You might need to transform the __powf4_massv to llvm.pow intrinsic at some place like PartiallyInlineLibCallsLegacyPass if the argument is 0.75

Maybe, I miss some background here and I just want to get a clear understanding of the reason why we have to do it in DAG. Thank you for your patience.

Jun 3 2020, 8:15 AM · Restricted Project, Restricted Project

Jun 2 2020

masoud.ataei added a comment to D80744: DAGCombiner optimization for pow(x,0.75) and pow(x,0.25) on double and single precision even in case massv function is asked.

What we are doing is as follows:
llvm.pow(IR) --> FPOW(ISD) --> __powf4_P8/9(ISD/IR)

It makes more sense to do it in the IR pass from what I see. And then, you can query the TargetLibraryInfoImpl::isFunctionVectorizable(name) instead of the option. Maybe, we can do it in ppc pass: PPCLowerMASSVEntries which did something like:

__sind2_massv --> __sind2_P9 for a Power9 subtarget.

Does it make sense ?

I agree in general it makes more sense to do this kind of conversions in an IR pass like PPCLowerMASSVEntries. This is what we are currently doing in LLVM but there is a problem here. If we change the llvm intrinsic to libcall earlier than a later optimization (like in DAGCombiner) the later optimization won't be triggered. In the case that I am proposing the change, if we have pow(x,0.75) in the code, the PPCLowerMASSVEntries pass will currently change it to __powf4_P* libcall then later in the DAGCombiner we will not get the optimization pow(x,0.75) --> sqrt(x)*sqrt(sqrt(x)). So that's a problem, because sqrt(x)*sqrt(sqrt(x)) is faster.

What I am proposing is to move the conversion for powf4 to late in the compiler pipeline. With this change we will we will get above optimization when exponent is 0.75 and MASSV calls otherwise.

If we expect the llvm.pow(x, 0.75) to be lowered as two sqrt and for others, they are libcall, can we just don't transform it as libcall for 0.75 in PPCLowerMASSVEntries? And it will be lowered as two sqrts in DAGCombine.

Jun 2 2020, 8:46 AM · Restricted Project, Restricted Project

Jun 1 2020

masoud.ataei added a comment to D80744: DAGCombiner optimization for pow(x,0.75) and pow(x,0.25) on double and single precision even in case massv function is asked.

What we are doing is as follows:
llvm.pow(IR) --> FPOW(ISD) --> __powf4_P8/9(ISD/IR)

It makes more sense to do it in the IR pass from what I see. And then, you can query the TargetLibraryInfoImpl::isFunctionVectorizable(name) instead of the option. Maybe, we can do it in ppc pass: PPCLowerMASSVEntries which did something like:

__sind2_massv --> __sind2_P9 for a Power9 subtarget.

Does it make sense ?

Jun 1 2020, 9:10 AM · Restricted Project, Restricted Project
masoud.ataei updated the diff for D80744: DAGCombiner optimization for pow(x,0.75) and pow(x,0.25) on double and single precision even in case massv function is asked.

Addressing the reviews

Jun 1 2020, 9:06 AM · Restricted Project, Restricted Project

May 28 2020

masoud.ataei created D80744: DAGCombiner optimization for pow(x,0.75) and pow(x,0.25) on double and single precision even in case massv function is asked.
May 28 2020, 10:56 AM · Restricted Project, Restricted Project

Apr 30 2020

masoud.ataei committed rGb4934ae44cf4: [VFDatabase] Testsuite for scalar functions are vector functions with VF =1 (authored by masoud.ataei).
[VFDatabase] Testsuite for scalar functions are vector functions with VF =1
Apr 30 2020, 12:55 PM
masoud.ataei closed D79124: [VFDatabase] Testsuite for scalar functions are vector functions with VF =1 .
Apr 30 2020, 12:54 PM · Restricted Project

Apr 29 2020

masoud.ataei updated the diff for D79124: [VFDatabase] Testsuite for scalar functions are vector functions with VF =1 .

Removing other dependencies to PowerPC machines in the test suits.

Apr 29 2020, 3:06 PM · Restricted Project
masoud.ataei created D79124: [VFDatabase] Testsuite for scalar functions are vector functions with VF =1 .
Apr 29 2020, 2:00 PM · Restricted Project
masoud.ataei added inline comments to D78054: [VFDatabase] Scalar functions are vector functions with VF =1.
Apr 29 2020, 1:28 PM · Restricted Project

Apr 28 2020

masoud.ataei updated the diff for D78054: [VFDatabase] Scalar functions are vector functions with VF =1.

Oh, sorry I missed these part in my last update.
Thanks Francesco,

Apr 28 2020, 7:28 AM · Restricted Project

Apr 27 2020

masoud.ataei updated the diff for D78054: [VFDatabase] Scalar functions are vector functions with VF =1.

Sorry, I was missing your suggestion of commenting above check on the test file.
Done.

Apr 27 2020, 2:34 PM · Restricted Project
masoud.ataei updated the diff for D78054: [VFDatabase] Scalar functions are vector functions with VF =1.

Reviews are addressed.
Thanks Francesco,

Apr 27 2020, 2:34 PM · Restricted Project
masoud.ataei added inline comments to D78054: [VFDatabase] Scalar functions are vector functions with VF =1.
Apr 27 2020, 2:01 PM · Restricted Project
masoud.ataei updated the diff for D78054: [VFDatabase] Scalar functions are vector functions with VF =1.

Thanks Francesco for the reviews.
The reviews are addressed.

Apr 27 2020, 11:51 AM · Restricted Project

Apr 24 2020

masoud.ataei added a comment to D78054: [VFDatabase] Scalar functions are vector functions with VF =1.

Reviews addressed.

Apr 24 2020, 2:05 PM · Restricted Project
masoud.ataei updated the diff for D78054: [VFDatabase] Scalar functions are vector functions with VF =1.

Addressing comments and adding unit tests.

Apr 24 2020, 2:05 PM · Restricted Project

Apr 22 2020

masoud.ataei updated the diff for D78054: [VFDatabase] Scalar functions are vector functions with VF =1.
Apr 22 2020, 11:58 AM · Restricted Project
masoud.ataei updated the summary of D78054: [VFDatabase] Scalar functions are vector functions with VF =1.
Apr 22 2020, 11:58 AM · Restricted Project

Apr 17 2020

masoud.ataei updated the diff for D78054: [VFDatabase] Scalar functions are vector functions with VF =1.

Assert is required for this test.

Apr 17 2020, 10:14 AM · Restricted Project

Apr 16 2020

masoud.ataei updated the diff for D78054: [VFDatabase] Scalar functions are vector functions with VF =1.

I am adding a test to my proposed change.

Apr 16 2020, 11:41 AM · Restricted Project

Apr 14 2020

masoud.ataei added a comment to D78054: [VFDatabase] Scalar functions are vector functions with VF =1.

It would be great if you could explain *why* it needs to set this, ideally with a test case. The existing code either does not use NeedToScalarize or initializes it with false. Technically we don't need to scalarize with VF==1, its already the scalar loop.

Apr 14 2020, 10:43 AM · Restricted Project

Apr 13 2020

masoud.ataei created D78054: [VFDatabase] Scalar functions are vector functions with VF =1.
Apr 13 2020, 2:07 PM · Restricted Project

Feb 28 2020

masoud.ataei added a comment to D75354: Add InjectTLIMappings pass to new pass manager.

@Whitney @fpetrogalli @pjeeva01:
Thank you guys for those comments. About the test, as @fpetrogalli said test for attribute generation is available llvm/test/Transforms/Util/add-TLI-mappings.ll for new pass manager. And I added the test to see if it is invoked in pipeline here in this patch. As @pjeeva01 said, I need to add a test to see if the vectorized function is called in presence of the vector library option.

Feb 28 2020, 9:55 AM · Restricted Project
masoud.ataei updated the summary of D75354: Add InjectTLIMappings pass to new pass manager.
Feb 28 2020, 8:14 AM · Restricted Project
masoud.ataei created D75354: Add InjectTLIMappings pass to new pass manager.
Feb 28 2020, 8:14 AM · Restricted Project

Dec 14 2018

masoud.ataei added inline comments to D54911: [compiler-rt][builtins][PowerPC] Add ___fixunstfti builtin compiler-rt method support for PowerPC .
Dec 14 2018, 12:58 PM