Page MenuHomePhabricator

zuban32 (Aleksandr Bezzubikov)
User

Projects

User does not belong to any projects.

User Details

User Since
Apr 29 2021, 6:43 AM (99 w, 3 d)

Recent Activity

Wed, Mar 1

zuban32 added inline comments to D144897: [SPIRV] fix several issues in builds with expensive checks.
Wed, Mar 1, 4:17 AM · Restricted Project, Restricted Project

Oct 4 2022

zuban32 added inline comments to D135106: [SPIRV] read kernel arg attributes from fuction/module metadata.
Oct 4 2022, 8:39 PM · Restricted Project, Restricted Project

Sep 14 2022

zuban32 accepted D133253: [SPIRV] add IR regularization pass.

OK, LGTM then

Sep 14 2022, 5:17 PM · Restricted Project, Restricted Project
zuban32 added inline comments to D133253: [SPIRV] add IR regularization pass.
Sep 14 2022, 12:29 AM · Restricted Project, Restricted Project

Sep 10 2022

zuban32 requested changes to D133253: [SPIRV] add IR regularization pass.

Could you pls specify a list of these small bug fixes, at the very high level at least? An ideal variant would be to not mix them up with this regularization stuff at all.

Sep 10 2022, 2:00 AM · Restricted Project, Restricted Project

Aug 31 2022

zuban32 added inline comments to D132648: [SPIRV] support builtin types and ExtInst selection.
Aug 31 2022, 11:48 PM · Restricted Project, Restricted Project

Aug 30 2022

zuban32 committed rG1fbc6b26a237: [SPIR-V] Use llvm::Optional for builtin lowering result. (authored by zuban32).
[SPIR-V] Use llvm::Optional for builtin lowering result.
Aug 30 2022, 11:26 PM · Restricted Project, Restricted Project
zuban32 closed D132802: [SPIR-V] Use llvm::Optional for builtin lowering result..
Aug 30 2022, 11:26 PM · Restricted Project, Restricted Project
zuban32 updated the diff for D132802: [SPIR-V] Use llvm::Optional for builtin lowering result..
  • clang-format
Aug 30 2022, 4:35 PM · Restricted Project, Restricted Project

Aug 28 2022

zuban32 retitled D132802: [SPIR-V] Use llvm::Optional for builtin lowering result. from [SPIR-V] Use std::optional for builtin lowering result. to [SPIR-V] Use llvm::Optional for builtin lowering result..
Aug 28 2022, 5:45 PM · Restricted Project, Restricted Project
zuban32 updated the diff for D132802: [SPIR-V] Use llvm::Optional for builtin lowering result..
  • Address comments
Aug 28 2022, 5:29 PM · Restricted Project, Restricted Project

Aug 27 2022

zuban32 updated the summary of D132802: [SPIR-V] Use llvm::Optional for builtin lowering result..
Aug 27 2022, 4:38 PM · Restricted Project, Restricted Project
zuban32 added reviewers for D132802: [SPIR-V] Use llvm::Optional for builtin lowering result.: iliya-diyachkov, mpaszkowski, konrad.trifunovic, andreytr, MaskRay, arsenm, rengolin.
Aug 27 2022, 4:33 PM · Restricted Project, Restricted Project
zuban32 requested review of D132802: [SPIR-V] Use llvm::Optional for builtin lowering result..
Aug 27 2022, 4:30 PM · Restricted Project, Restricted Project

Aug 12 2022

zuban32 accepted D131716: X86: Don't fold TEST into ADD ...@GOTTPOFF/GOTNTPOFF/INDNTPOFF.

LGTM regarding removal of unrelated SPIR-V code

Aug 12 2022, 10:16 AM · Restricted Project, Restricted Project

Aug 11 2022

zuban32 requested changes to D131716: X86: Don't fold TEST into ADD ...@GOTTPOFF/GOTNTPOFF/INDNTPOFF.
Aug 11 2022, 4:28 PM · Restricted Project, Restricted Project

Jul 21 2022

zuban32 added a comment to D129730: [SPIRV] add PrepareFunctions pass and update other passes.

To me it seems as an unnecessarily huge change. Can't it be split, at least keep PrepareFunctions and other minor changes separate?

Well, it looks big, but comparable to the patches from the initial series (and even smaller then some of them). By the way, the changes in most files are quite minor. Only 5 files are significantly changed (SPIRVAsmPrinter.cpp, SPIRVCallLowering.cpp, SPIRVGlobalRegistry.cpp, SPIRVInstructionSelector.cpp, SPIRVModuleAnalysis.cpp), and just one new pass is added.

It's well tested and provides a good improvement in the stability and functionality of the SPIRV backend, so I wanted to introduce it before the 15th release. If the size is not a big issue and you don't mind, I will commit it as it is.

Jul 21 2022, 1:08 PM · Restricted Project, Restricted Project
zuban32 added a comment to D129730: [SPIRV] add PrepareFunctions pass and update other passes.

To me it seems as an unnecessarily huge change. Can't it be split, at least keep PrepareFunctions and other minor changes separate?

Jul 21 2022, 7:53 AM · Restricted Project, Restricted Project

Jul 7 2022

zuban32 committed rGfa2a7a25c989: [SPIR-V] Introduce SPIR-V global entities tracking and deduplication infra. (authored by zuban32).
[SPIR-V] Introduce SPIR-V global entities tracking and deduplication infra.
Jul 7 2022, 8:15 AM · Restricted Project, Restricted Project
zuban32 closed D128471: [SPIR-V] Introduce SPIR-V global entities tracking and deduplication infra..
Jul 7 2022, 8:15 AM · Restricted Project, Restricted Project

Jul 2 2022

zuban32 added a comment to D128471: [SPIR-V] Introduce SPIR-V global entities tracking and deduplication infra..

@iliya-diyachkov FYI there're 6 more passing tests which are not upstreamed yet, but I've not added them here since they're irrelevant to the current changes

Jul 2 2022, 4:59 PM · Restricted Project, Restricted Project
zuban32 updated the diff for D128471: [SPIR-V] Introduce SPIR-V global entities tracking and deduplication infra..
  • Fix potential failure, add 2 newly passed LITs
Jul 2 2022, 4:56 PM · Restricted Project, Restricted Project
zuban32 updated the diff for D128471: [SPIR-V] Introduce SPIR-V global entities tracking and deduplication infra..

Restore

Jul 2 2022, 4:48 PM · Restricted Project, Restricted Project
zuban32 updated the diff for D128471: [SPIR-V] Introduce SPIR-V global entities tracking and deduplication infra..

Fix potential failure, add 2 newly passed LITs

Jul 2 2022, 4:30 PM · Restricted Project, Restricted Project

Jun 23 2022

zuban32 added reviewers for D128471: [SPIR-V] Introduce SPIR-V global entities tracking and deduplication infra.: iliya-diyachkov, MaskRay, rengolin, arsenm, konrad.trifunovic, mpaszkowski, andreytr.
Jun 23 2022, 3:15 PM · Restricted Project, Restricted Project
zuban32 requested review of D128471: [SPIR-V] Introduce SPIR-V global entities tracking and deduplication infra..
Jun 23 2022, 3:11 PM · Restricted Project, Restricted Project

May 17 2022

zuban32 accepted D125404: [SPIRV] Add simple tests to improve test coverage.

LGTM

May 17 2022, 8:28 AM · Restricted Project, Restricted Project

May 16 2022

zuban32 added inline comments to D125404: [SPIRV] Add simple tests to improve test coverage.
May 16 2022, 1:39 PM · Restricted Project, Restricted Project
zuban32 added inline comments to D125404: [SPIRV] Add simple tests to improve test coverage.
May 16 2022, 10:35 AM · Restricted Project, Restricted Project

May 11 2022

zuban32 requested changes to D125404: [SPIRV] Add simple tests to improve test coverage.

To me this subset of tests looks very counter-intuitive, I'd strongly prefer to have LITs related commits formed by some tests' common properties, e.g. '[SPIR-V] Add simple transcoding tests'. What was your motivation behind this particular subset, are these the only tests currently passing?

May 11 2022, 2:28 PM · Restricted Project, Restricted Project

Apr 20 2022

zuban32 added a reviewer for D116464: [SPIRV 5/6] Add LegalizerInfo, InstructionSelector and utilities: nikic.
Apr 20 2022, 8:57 AM · Restricted Project, Restricted Project

Feb 14 2022

zuban32 added a comment to D116465: [SPIRV 6/6] Add the module analysis pass and the simplest tests.

@arsenm, when you suggested moving GlobalTypesAndRegNum' operations to AsmPrinter, did you implied that we need to insert SPIRVDummyGlobalPass after all SPIRVGISel passes to have whole MIR at the first run of AsmPrinter? Or is there another approach (or some pass configuration) to achieve the correct processing of the whole MIR in AsmPrinter?

Feb 14 2022, 2:11 PM · Restricted Project, Restricted Project

Feb 9 2022

zuban32 added inline comments to D116465: [SPIRV 6/6] Add the module analysis pass and the simplest tests.
Feb 9 2022, 4:35 PM · Restricted Project, Restricted Project

Feb 8 2022

zuban32 added inline comments to D116465: [SPIRV 6/6] Add the module analysis pass and the simplest tests.
Feb 8 2022, 7:18 AM · Restricted Project, Restricted Project

Feb 3 2022

zuban32 added a comment to D116465: [SPIRV 6/6] Add the module analysis pass and the simplest tests.

And as far as I understand your suggestion this de-duplicated variant will break the verifier (after generation):

func1
  %0 = OpTypeInt 32
  %1 = OpTypeInt 64
  ...
  %4 = OpIAdd %1 %2 %3
  ...
func2
  // (no definition of %1)
  ... 
  %4 = OpIMul %1 %2 %3
  ...

No, I'm suggestion you emit on every function (as is functionally correct) but reuse the same ID, and keep that same ID on global_function.

The singleton is just caching the VReg numbers, not the actual MNodes.

Code emission will be the same as what you have today after that.

I agree that this is a natural desire to use some caching in this situation. But I join Alexander in the question how the proposed caching would work at module (cross-function) level? As I understand we can define %type1 register in func1 and refer to the register inside func1, but it's illegal to refer %type1 in func2 without a local definition of %type1 (let me remind that in SPIR-V the type definition is a regular MIR instruction which produces %type register).

By just re-declaring each used type/constant in each subsequent function with the same VReg ID.

In your example:

After generation:

func1
  %0 = OpTypeInt 32
  %1 = OpTypeInt 64
  ...
  %4 = OpIAdd %1 %2 %3
  ...
func2
  %0 = OpTypeInt 32
  %1 = OpTypeInt 64
  ...
  %4 = OpIMul %1 %2 %3
  ...

After GlobalTypesAndRegNumPass:

global_func:
  %0 = OpTypeInt 32
  %1 = OpTypeInt 64
...
func1
  %0 = OpTypeInt 32
  %1 = OpTypeInt 64
  ...
  %4 = OpIAdd %1 %2 %3
  ...
func2
  %0 = OpTypeInt 32
  %1 = OpTypeInt 64
  ...
  %4 = OpIMul %1 %2 %3
  ...

To have the same MIR in your second example, straight from generation, you have to follow these steps:

  • First time you see, generate a new type (OpTypeInt 32) with a new auto-increment ID, cache the ID for the original type, emit on global_func and *also* on the function you're generating.
  • Every other time you see, get the cached VReg ID and re-emit (a new MNode) using that same ID.

In your example:

  • Inside func1 you generate a new type (OpTypeInt 32) and call it %0, cache %0 for i32, emit on global_func and *also* on func1.
  • Inside func2 you get the cached version (of i32 as %0) and generate it *again* only on func2 (on a new Node, using %0). global_func already have it's the same VReg number on all previous functions.

If you use a global auto-increment next_id(), there's no way VReg IDs for operations in any function will clash with previously defined types and constants (in previous functions).

The VReg numbers on the later functions will be (numerically) large, but the number of different IDs in each function will still be the same, so code generation should be equally efficient.

Feb 3 2022, 8:49 AM · Restricted Project, Restricted Project

Feb 2 2022

zuban32 added a comment to D116465: [SPIRV 6/6] Add the module analysis pass and the simplest tests.

I didn't mean the duplicated types/constants would survive into codegen, but I get your meaning, it would have to be commoned up at some point.

So, what about a singleton emitter for types and constants?

Instead of just get_next_id() you could have a get_type(...) and get_constant(...) that returns a cached MIR node if it has created a similar one already, or creates a new one, caches and returns it.

In this way, there's only ever one copy of each and they're only generated once, with no need to deduplicate.

The only issue with this implementation is that types, constants, functions, instructions will all have intertwined IDs, but never refer to larger IDs, by design.

When emitting "int foo()", if int hadn't been emitted before, it'd get the next id (say, %23) and then foo would get %24 and would refer to %23 as its type.

Eventually, all types used in the program would have been emitted already and be brought up from the cache every time.

This way, there's no need to renumber or common up and the space is still optimal.

What am I missing?

Feb 2 2022, 2:38 PM · Restricted Project, Restricted Project
zuban32 added a comment to D116465: [SPIRV 6/6] Add the module analysis pass and the simplest tests.

Then let me describe the problem in more details, I'd like us all to be on the same page with this.

Awesome! This is much clearer to me, thanks!

The latter seems to be a minor problem - the reason for it to exist is SPIRV binary format requiring global numbering of all the defined values, i.e. if there're 2 functions each containing only 1 instruction then these instructions can't be both numbered %0.

Do you have a maximum ID? If not, than in theory, you just need a static global auto-increment ID (like a get_next_id() on some target emission class) and off you go emitting globally unique IDs.

The former one attempts to solve a bigger problem. Basically the SPIRV binary is structured as following:

...
types, constants
...
functions definitions

I see, so your "global" function contain the type and constant declarations, that the functions use in their own declarations.

So, you're not "moving" anything, just creating a placeholder for those definitions and indexing them.

in our approach each MachineFunction has its own copies of type decls and constants (and other stuff like that) during the whole translation process, but eventually they need to be deduplicated and 'globalized'. That can be done either on MIR level or during binary emission, but it has done be done anyway.

IIUC, they're in each function because you generate for each function you pass through, so:

Feb 2 2022, 1:52 PM · Restricted Project, Restricted Project
zuban32 added a comment to D116465: [SPIRV 6/6] Add the module analysis pass and the simplest tests.

@rengolin what would be considered as a proper implementation?

I don't think I have quite understood what the problem is, tbh.

If this is just some global value numbering, why is it being done so late (ie, not in IR)?

If this is a SPIR-V hardware requirement, that needs a specific guaranteed shape that cannot rely on the state of IR->MIR conversions (especially the generic parts), then there may not be another way out.

But I'll leave for others to express their opinions, as I'm not familiar with the problem.

Feb 2 2022, 9:49 AM · Restricted Project, Restricted Project
zuban32 added a comment to D116465: [SPIRV 6/6] Add the module analysis pass and the simplest tests.

I'm still not convinced that these passes are the right way to go, but I won't block if others think it has merit, even if just temporary, until the proper implementation emerges.

We don't need these patches to be perfect, just good enough to generate some instructions and tell us that the direction the target is going is in sync with what we think should be.

If this is something that can be changed later, I'd be fine with it.

@arsenm @MaskRay

Feb 2 2022, 8:43 AM · Restricted Project, Restricted Project

Jan 11 2022

zuban32 added inline comments to D115786: [SPIR-V 2/6] Add SPIRV{InstrFormats,InstrInfo,RegisterInfo,RegisterBanks...}.td.
Jan 11 2022, 2:04 PM · Restricted Project, Restricted Project

Jan 10 2022

zuban32 added inline comments to D116464: [SPIRV 5/6] Add LegalizerInfo, InstructionSelector and utilities.
Jan 10 2022, 12:14 PM · Restricted Project, Restricted Project
zuban32 added inline comments to D116464: [SPIRV 5/6] Add LegalizerInfo, InstructionSelector and utilities.
Jan 10 2022, 11:47 AM · Restricted Project, Restricted Project
zuban32 added inline comments to D116464: [SPIRV 5/6] Add LegalizerInfo, InstructionSelector and utilities.
Jan 10 2022, 11:45 AM · Restricted Project, Restricted Project
zuban32 added inline comments to D116464: [SPIRV 5/6] Add LegalizerInfo, InstructionSelector and utilities.
Jan 10 2022, 10:39 AM · Restricted Project, Restricted Project

Dec 31 2021

zuban32 updated subscribers of D116462: [SPIRV 3/6] Add MC layer, object file support and InstPrinter.

Iliya, since this is the 3rd patch of a total of 6, do I understand
correctly that the second one is https://reviews.llvm.org/D115786 from
Michal?

Dec 31 2021, 6:36 PM · Restricted Project, Restricted Project

Sep 21 2021

zuban32 added a comment to D108375: [GlobalISel] Support ConstantAsMetadata in IRTranslator.

Ping

Sep 21 2021, 7:07 AM · Restricted Project

Sep 17 2021

zuban32 added a comment to D108375: [GlobalISel] Support ConstantAsMetadata in IRTranslator.

Someone pls commit this, I don't have an access

Sep 17 2021, 7:27 AM · Restricted Project

Sep 15 2021

zuban32 updated the diff for D108375: [GlobalISel] Support ConstantAsMetadata in IRTranslator.

Rebased, added a test.

Sep 15 2021, 4:58 PM · Restricted Project

Sep 2 2021

zuban32 added a comment to D108375: [GlobalISel] Support ConstantAsMetadata in IRTranslator.

Can someone pls commit this? I don't have an access to do that myself

Sep 2 2021, 12:49 PM · Restricted Project

Aug 26 2021

zuban32 abandoned D101538: [GlobalISel][IRTranslator] Make translate() methods virtual..
Aug 26 2021, 9:45 AM · Restricted Project
zuban32 added a comment to D108375: [GlobalISel] Support ConstantAsMetadata in IRTranslator.

Can someone pls commit this? I don't have an access to do that myself

Aug 26 2021, 9:45 AM · Restricted Project
zuban32 added a comment to D108375: [GlobalISel] Support ConstantAsMetadata in IRTranslator.

Gentle ping

Aug 26 2021, 7:48 AM · Restricted Project

Aug 25 2021

zuban32 updated the diff for D108375: [GlobalISel] Support ConstantAsMetadata in IRTranslator.

Fix clang-tidy warning

Aug 25 2021, 4:48 PM · Restricted Project

Aug 20 2021

zuban32 added a comment to D108375: [GlobalISel] Support ConstantAsMetadata in IRTranslator.

Can you add a testcase?

Aug 20 2021, 4:50 AM · Restricted Project

Aug 19 2021

zuban32 added reviewers for D108375: [GlobalISel] Support ConstantAsMetadata in IRTranslator: paquette, aemerson, arsenm.
Aug 19 2021, 7:47 AM · Restricted Project
zuban32 requested review of D108375: [GlobalISel] Support ConstantAsMetadata in IRTranslator.
Aug 19 2021, 7:44 AM · Restricted Project

Jul 13 2021

zuban32 added a comment to D101538: [GlobalISel][IRTranslator] Make translate() methods virtual..

Let me explain my problem with a short example. Assume we have the following IR:

Jul 13 2021, 12:13 PM · Restricted Project

Jul 8 2021

zuban32 added a comment to D101538: [GlobalISel][IRTranslator] Make translate() methods virtual..

@qcolombet @arsenm @foad this change is already abandoned, but my question seems directly related to it.

Jul 8 2021, 5:08 PM · Restricted Project

May 21 2021

zuban32 added a comment to D101538: [GlobalISel][IRTranslator] Make translate() methods virtual..

I wouldn't be so sure about that. One particular usecase I need this for is a bitcast between pointer types:

%1 = bitcast i8* %0 to %struct.ST*
In our target they're both of p0 LLTs, so the default method eliminates this instruction while in our target it's necessary to keep it.

That sounds like you're abusing the IR here. Shouldn't you have a different addrspace here?

FWIW, when we discussed the design of GISel internally originally we explicitly chose not to allow virtual overloading of the IR translation steps because that step follow what is represented in the IR.
To make a parallel with SDISel, this is as if we would allow SDISelBuilder to be target specific.

Bottom line, for just this one use case I feel that it doesn't make sense to allow virtual method everywhere yet.

You could workaround that with introducing a pseudo/intrisinc in codegen prepare for instance.

My 2 cents.

Cheers,
-Quentin

May 21 2021, 7:23 AM · Restricted Project

May 19 2021

zuban32 added a comment to D101538: [GlobalISel][IRTranslator] Make translate() methods virtual..

Won't this problem go away when we introduce FP LLTs?

I wouldn't be so sure about that. One particular usecase I need this for is a bitcast between pointer types:

%1 = bitcast i8* %0 to %struct.ST*

In our target they're both of p0 LLTs, so the default method eliminates this instruction while in our target it's necessary to keep it.

What are you going to do when IR switches to opaque pointers? https://llvm.org/docs/OpaquePointers.html

May 19 2021, 8:50 AM · Restricted Project

May 18 2021

zuban32 added a comment to D101538: [GlobalISel][IRTranslator] Make translate() methods virtual..

Is this really necessary? Our preference is to use custom combines/lowering passes, or custom legalization, to do this.

Well, our usecase might not be that canonical, let me briefly explain. For example, translating a bitcast - default IRTranslator implementation turns it into a COPY when LLTs of src and dst are the same. In our target (SPIR-V) LLTs do not matter that much, the typeinfo is represented with some pseudo instructions, so we run into this situation quite often. And losing a bitcast in the translator doesn't work for us as it'd obviously be quite difficult to recover it.

Won't this problem go away when we introduce FP LLTs?

May 18 2021, 11:35 AM · Restricted Project

May 17 2021

zuban32 added a comment to D101538: [GlobalISel][IRTranslator] Make translate() methods virtual..

@aemerson @foad if no one has any idea how to make CRTP approach feasible here, then I'd suggest to merge this, I have no write access though

May 17 2021, 9:11 AM · Restricted Project

May 13 2021

zuban32 added a comment to D101538: [GlobalISel][IRTranslator] Make translate() methods virtual..

Is this really necessary? Our preference is to use custom combines/lowering passes, or custom legalization, to do this.

Well, our usecase might not be that canonical, let me briefly explain. For example, translating a bitcast - default IRTranslator implementation turns it into a COPY when LLTs of src and dst are the same. In our target (SPIR-V) LLTs do not matter that much, the typeinfo is represented with some pseudo instructions, so we run into this situation quite often. And losing a bitcast in the translator doesn't work for us as it'd obviously be quite difficult to recover it.

I see. Is this a widespread issue with a lot of the translation functions or just bitcasts? If it's just bitcast you could just make that virtual so we don't have to pay the virtual function call overhead every translate. If not, then I'm ok with doing this.

How about doing it with https://en.wikipedia.org/wiki/Curiously_recurring_template_pattern#Static_polymorphism to avoid the virtual function call overhead?

May 13 2021, 3:18 PM · Restricted Project

May 11 2021

zuban32 added a comment to D101538: [GlobalISel][IRTranslator] Make translate() methods virtual..

Is this really necessary? Our preference is to use custom combines/lowering passes, or custom legalization, to do this.

Well, our usecase might not be that canonical, let me briefly explain. For example, translating a bitcast - default IRTranslator implementation turns it into a COPY when LLTs of src and dst are the same. In our target (SPIR-V) LLTs do not matter that much, the typeinfo is represented with some pseudo instructions, so we run into this situation quite often. And losing a bitcast in the translator doesn't work for us as it'd obviously be quite difficult to recover it.

I see. Is this a widespread issue with a lot of the translation functions or just bitcasts? If it's just bitcast you could just make that virtual so we don't have to pay the virtual function call overhead every translate. If not, then I'm ok with doing this.

May 11 2021, 12:34 PM · Restricted Project
zuban32 added a comment to D101538: [GlobalISel][IRTranslator] Make translate() methods virtual..

Is this really necessary? Our preference is to use custom combines/lowering passes, or custom legalization, to do this.

May 11 2021, 8:58 AM · Restricted Project

May 7 2021

zuban32 added a comment to D101538: [GlobalISel][IRTranslator] Make translate() methods virtual..

Gentle ping

May 7 2021, 3:35 PM · Restricted Project

Apr 29 2021

zuban32 added reviewers for D101538: [GlobalISel][IRTranslator] Make translate() methods virtual.: aemerson, qcolombet, t.p.northover, paquette, foad.
Apr 29 2021, 5:18 PM · Restricted Project
zuban32 updated the diff for D101538: [GlobalISel][IRTranslator] Make translate() methods virtual..

Rebased

Apr 29 2021, 11:54 AM · Restricted Project
zuban32 requested review of D101538: [GlobalISel][IRTranslator] Make translate() methods virtual..
Apr 29 2021, 7:40 AM · Restricted Project