Commit Graph

23 Commits

Author SHA1 Message Date
Diego Novillo
cbd1fa6c42
Simplify logic to decide whether CCP modified the IR (#3997)
Simplify logic to decide whether CCP modified the IR.

The previous attempts at fixing this issue relied on marking the IR
changed only when CCP was able to fold an instruction during
propagation (https://github.com/KhronosGroup/SPIRV-Tools/pull/3799,
https://github.com/KhronosGroup/SPIRV-Tools/pull/3732).

Those fixes missed the case described in
https://github.com/KhronosGroup/SPIRV-Tools/issues/3991.  In this case,
the folder never actually succeeds in folding the instruction, but it
does create constants in the process.

Fixed with this change.
2020-10-28 10:18:34 -04:00
Diego Novillo
286b3095dd
Properly mark IR changed if instruction folder creates more than one constant. (#3799)
In #3636, I missed that the instruction folder may create more than a
single constant per call.  Since CCP was only checking whether one
constant had been created after folding, it was wrongly thinking that
the IR had not changed.

Fixes #3738.
2020-09-14 09:00:38 -04:00
Diego Novillo
b79773a35d
CCP should mark IR changed if it created new constants. (#3732)
CCP should mark IR changed if it created new constants.

This fixes #3636.

When CCP is simulating statements, it will sometimes successfully fold
an instruction, which laters switches to varying.  The initial fold of
the instruction may generate a new constant K.

The problem we were running into is when K never gets propagated to the
IR.  Its definition will still exist, so CCP should mark the IR modified
in this case.

In fixing this bug, I noticed that an existing test was suffering from
the same bug.  The change also makes PassTest::SinglePassRunAndMatch()
return the result from the pass, so that we can check that the pass
marks the IR modified in this case.
2020-08-20 16:48:11 -04:00
Jaebaek Seo
5543d5faa9
Debug info preservation in ccp pass (#3420)
* Debug info preservation in ccp pass

For constant propagation, the ccp pass already replaces the result id of
a value with a result id of the corresponding constant value. As a part
of the replacement, it correctly updates the operands of
DebugValue/DebugDeclare as well. Since we do not have to any addition
work other than the ccp pass itself, this commit just adds unit tests to
check the debug information preservation.
2020-06-12 10:54:06 -04:00
David Neto
9f188e3374 Assembler: Can't set an ID in instruction without result ID (#2852)
Fix tests that violated this rule.

Fixes #2257
2019-09-11 13:15:25 -04:00
Steven Perron
9d29c37ac5
Removing decorations when doing constant propagation. (#2444)
In constant propagation, decoration are transfered from the original
expression to the constant that will replace it.  This can be wrong
because there are no decorations that apply to constants.  We choose to
simply delete the decorations.

Fixes #2441
2019-03-13 10:40:49 -04:00
Steven Perron
c4c68712c4
Make EFFCEE required (#1943)
Fixes #1912.

Remove the non-effcee build as EFFCEE is now required.
2018-10-04 10:00:11 -04:00
Alan Baker
a77bb2e54b Add validation for execution modes
* Check rules from Execution Mode tables, 2.16.2 and the Vulkan
environment spec

* Allows MeshNV execution model with the following execution modes
 * LocalSize, LocalSizeId, OutputPoints and OutputVertices
 * Done to not break their validation
2018-10-02 10:22:23 -04:00
dan sinclair
eda2cfbe12
Cleanup includes. (#1795)
This Cl cleans up the include paths to be relative to the top level
directory. Various include-what-you-use fixes have been added.
2018-08-03 15:06:09 -04:00
dan sinclair
2cce2c5b97
Move tests into namespaces (#1689)
This CL moves the test into namespaces based on their directories.
2018-07-11 09:24:49 -04:00
Alan Baker
824625760b Fixes #1361. Mark all non-constant global values as varying in CCP
* Also mark function parameters as varying
* Conservatively mark assignment instructions as varying if any input is
varying after attempting to fold
* Added a test to catch this case
2018-03-01 15:24:41 -05:00
Alan Baker
01760d2f0f Fixes #1338. Handle OpConstantNull in branch/switch conditions
* No longer assume the branch/switch condition must be bool or int
constants (respectively)
* Added a couple unit tests for each case
2018-02-21 10:22:39 -05:00
Alan Baker
c3f34d8bf3 Fixes #1300. Adding checks for bad CCP transitions and unsettled values
* Now track propagation status and assert on bad statuses
 * Added helper methods to access instruction propagation status
* Modified the phi meet operator to properly reflect the paper it is
based on
* Modified SSA edge addition so that all edge are added, but only on
state changes
* Fixed a bug in instruction simulation where interesting conditional
branches would not mark the interesting edge as executed
 * Added a test to catch this bug
* Added an ostream operator for SSAPropagator::PropStatus
2018-02-18 19:41:34 -05:00
Steven Perron
3756b387f3 Get CCP to use the constant floating point rules.
Fixes #1311
2018-02-16 13:49:47 -05:00
Alan Baker
229ebc0665 Fixes #1295. Mark undef values as varying in ccp.
* Undef now marked as varying in ccp
 * this prevents incorrect meet operations since phis were always not
 interesting
* added a test to catch the bug
2018-02-14 10:21:26 -05:00
Alan Baker
1b46f7ecad Fixes in CCP for #1228
* Forces traversal of phis if the def has changed to varying
* Mark a phi as varying if all incoming values are varying
* added a test to catch the bug
2018-01-29 15:12:05 -05:00
Steven Perron
6cc772c3ce Skip SpecConstants in CCP.
At the moment specialization constants look like constants to ccp.  This
causes a problem because they are handled differently by the constant
manager.

I choose to simply skip over them, and not try to add them to the value
table.  We can do specialization before ccp if we want to be able to
propagate these values.

Fixes #1199.
2018-01-15 09:53:23 -05:00
Steven Perron
24f9947050 Move initialization of the const mgr to the constructor.
The current code expects the users of the constant manager to initialize
it with all of the constants in the module.  The problem is that you do
not want to redo the work multiple times.  So I decided to move that
code to the constructor of the constant manager.  This way it will
always be initialized on first use.

I also removed an assert that expects all constant instructions to be
successfully mapped.  This is because not all OpConstant* instruction
can map to a constant, and neither do the OpSpecConstant* instructions.

The real problem is that an OpConstantComposite can contain a member
that is OpUndef.  I tried to treat OpUndef like OpConstantNull, but this
failed because an OpSpecConstantComposite with an OpUndef cannot be
changed to an OpConstantComposite.  Since I feel this case will not be
common, I decided to not complicate the code.

Fixes #1193.
2018-01-12 13:53:21 -05:00
Diego Novillo
e5560d64de Fix constant propagation of induction variables.
This fixes https://github.com/KhronosGroup/SPIRV-Tools/issues/1143.
When an instruction transitions from constant to bottom (varying) in the
lattice, we were telling the propagator that the instruction was
varying, but never updating the actual value in the values table.

This led to incorrect value substitutions at the end of propagation.

The patch also re-enables CCP in -O and -Os.
2018-01-08 15:34:35 -05:00
Diego Novillo
716718a5e9 Fix infinite simulation cycles in SSA propagator.
This fixes https://github.com/KhronosGroup/SPIRV-Tools/issues/1159.  I
had missed a nuance in the original algorithm.  When simulating Phi
instructions, the SSA edges out of a Phi instruction should never be
added to the list of edges to simulate.

Phi instructions can be in SSA def-use cycles with other Phi
instructions.  This was causing the propagator to fall into an infinite
loop when the same def-use edge kept being added to the queue.

The original algorithm in the paper specifically separates the visit of
a Phi instruction vs the visit of a regular instruction.  This fix makes
the implementation match the original algorithm.
2018-01-05 10:29:39 -05:00
Diego Novillo
5f100789fb Handle execution termination instructions when building edges.
This fixes issue https://github.com/KhronosGroup/SPIRV-Tools/issues/1153.

When building CFG edges, edges out of a OpKill and OpUnreachable
instruction should be directed to the CFG's pseudo exit block.
2018-01-03 15:25:03 -05:00
Diego Novillo
135150a1a8 Do not insert Phi nodes in CCP propagator.
In CCP we should not need to insert Phi nodes because CCP never looks at
loads/stores.  This required adjusting two tests that relied on Phi
instructions being inserted.  I changed the tests to have the Phi
instructions pre-inserted.

I also added a new test to make sure that CCP does not try to look
through stores and loads.

Finally, given that CCP does not handle loads/stores, it's better to run
mem2reg before it.  I've changed the -O/-Os schedules to run local
multi-store elimination before CCP.

Although this is just an efficiency fix for CCP, it is
also working around a bug in Phi insertion.  When Phi instructions are
inserted, they are never associated a basic block.  This causes a
segfault when the propagator tries to lookup CFG edges when analyzing
Phi instructions.
2018-01-03 15:12:25 -05:00
Diego Novillo
4ba9dcc8a0 Implement SSA CCP (SSA Conditional Constant Propagation).
This implements the conditional constant propagation pass proposed in

Constant propagation with conditional branches,
Wegman and Zadeck, ACM TOPLAS 13(2):181-210.

The main logic resides in CCPPass::VisitInstruction.  Instruction that
may produce a constant value are evaluated with the constant folder. If
they produce a new constant, the instruction is considered interesting.
Otherwise, it's considered varying (for unfoldable instructions) or
just not interesting (when not enough operands have a constant value).

The other main piece of logic is in CCPPass::VisitBranch.  This
evaluates the selector of the branch.  When it's found to be a known
value, it computes the destination basic block and sets it.  This tells
the propagator which branches to follow.

The patch required extensions to the constant manager as well. Instead
of hashing the Constant pointers, this patch changes the constant pool
to hash the contents of the Constant.  This allows the lookups to be
done using the actual values of the Constant, preventing duplicate
definitions.
2017-12-21 14:29:45 -05:00