Skip to content

Add undef for inlined void function#3720

Merged
s-perron merged 4 commits intoKhronosGroup:masterfrom
s-perron:i3704
Aug 24, 2020
Merged

Add undef for inlined void function#3720
s-perron merged 4 commits intoKhronosGroup:masterfrom
s-perron:i3704

Conversation

@s-perron
Copy link
Copy Markdown
Collaborator

It is possible that the result of a void function call is used. In case
it is used, we need something that still defines its id after inlining.
We add an undef for that purpose.

Fixes #3704

@s-perron s-perron requested a review from dnovillo August 17, 2020 20:30
It is possible that the result of a void function call is used.  In case
it is used, we need something that still defines its id after inlining.
We use an undef for that purpose.

Fixes KhronosGroup#3704
Copy link
Copy Markdown
Collaborator

@dneto0 dneto0 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At first I questioned whether doing OpCopyObject on a void argument is valid.

From the terms section:

Object: An instantiation of a non-void type, either as the Result <id> of an operation, or created through OpVariable.

That suggests that OpCopyObject shouldn't allow a void argument.
But the instruction is explicit about it.

Result Type must equal Operand type. There are no other restrictions on the types.

// Even though it is very unlikely, it is possible that the result id of
// the void-function call is used, so we need to generate an instruction
// with that result id.
std::unique_ptr<Instruction> undef_inst(
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OpUndef's are preferred to be in the constants section, not in function bodies.
This is ok. But we should consider this the deprecated way.

@dneto0
Copy link
Copy Markdown
Collaborator

dneto0 commented Aug 17, 2020

Tests are failing because some expectations were not updated.

For an improved MR:

  • only generate the undef value if the return value has a use
  • generate the undef value in the constants section of the module, rather than in the function body.

@s-perron
Copy link
Copy Markdown
Collaborator Author

only generate the undef value if the return value has a use

That is hard because we do not keep the def-use changes valid while inlining. We would have to keep rebuilding the def-use chains to do that, which would cost more compile time that I would like to use. I prefer to get get DCE to delete the undef if it is not used.

generate the undef value in the constants section of the module, rather than in the function body.

That is doable.

@s-perron s-perron merged commit 3f8501d into KhronosGroup:master Aug 24, 2020
@s-perron s-perron deleted the i3704 branch August 24, 2020 19:09
dneto0 pushed a commit to dneto0/SPIRV-Tools that referenced this pull request Sep 14, 2024
Roll third_party/glslang/ 983698b..517f39e (1 commit)

KhronosGroup/glslang@983698b...517f39e

$ git log 983698b..517f39e --date=short --no-merges --format='%ad %ae %s'
2020-08-26 jmadill Suppress two override suggestion warnings.

Created with:
  roll-dep third_party/glslang

Roll third_party/googletest/ 1e315c5b1..df6b75949 (1 commit)

google/googletest@1e315c5...df6b759

$ git log 1e315c5b1..df6b75949 --date=short --no-merges --format='%ad %ae %s'
2020-08-26 absl-team Googletest export

Created with:
  roll-dep third_party/googletest

Roll third_party/spirv-tools/ 4dd1223..8a0ebd4 (14 commits)

KhronosGroup/SPIRV-Tools@4dd1223...8a0ebd4

$ git log 4dd1223..8a0ebd4 --date=short --no-merges --format='%ad %ae %s'
2020-08-31 jaebaek Correctly replace debug lexical scope of instruction (KhronosGroup#3718)
2020-08-28 afdx spirv-fuzz: Remove opaque pointer design pattern (KhronosGroup#3755)
2020-08-27 stefanomil spirv-fuzz: Create synonym via OpPhi and existing synonyms (KhronosGroup#3701)
2020-08-27 stefanomil Add LoopNestingDepth function to StructuredCFGAnalysis (KhronosGroup#3754)
2020-08-27 afdx spirv-fuzz: Do not make synonyms of void result ids (KhronosGroup#3747)
2020-08-26 greg Do not register DebugFunction for functions optimized away. (KhronosGroup#3749)
2020-08-26 jaebaek Handle DebugScope in compact-ids pass (KhronosGroup#3724)
2020-08-26 afdx spirv-fuzz: Overflow ids (KhronosGroup#3734)
2020-08-25 greg Fix DebugNoScope to not output InlinedAt operand. (KhronosGroup#3748)
2020-08-25 vasniktel spirv-fuzz: Split the fact manager into multiple files (KhronosGroup#3699)
2020-08-25 andreperezmaselco.developer spirv-fuzz: Add inline function transformation (KhronosGroup#3517)
2020-08-25 vasniktel spirv-fuzz: Fix MaybeGetZeroConstant (KhronosGroup#3740)
2020-08-24 greg Fix SSA-rewrite to remove DebugDeclare for variables without loads (KhronosGroup#3719)
2020-08-24 stevenperron Add undef for inlined void function (KhronosGroup#3720)

Created with:
  roll-dep third_party/spirv-tools
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

spirv-opt: InlineExhaustivePass doesn't handle OpCopyObject of void

2 participants