Commit Graph

19 Commits

Author SHA1 Message Date
Nathan Gauër
1728c1d40a
spirv-opt: fix copy-propagate-arrays index opti on structs. (#4891)
* spirv-opt: fix copy-propagate-arrays index opti on structs.

As per SPIR-V spec:
OpAccessChain indices must be OpConstant when indexing into a structure.

This optimization tried to remove load cascade. But in some scenario
failed:

```c
cbuffer MyStruct {
    uint my_field;
};

uint main(uint index) {
    const uint my_array[1] = { my_field };
    return my_array[index]
}
```

This is valid as the struct is indexed with a constant index, and then
the array is indexed using a dynamic index.
The optimization would consider the local array to be useless and
generated a load directly into the struct.

* spirv-opt: prevent creation of unused instructions

Copy-propagate-arrays optimization pass would create unused constants,
even if the optimization not completed.
This was caused by the way we handled OpAccessChain squashing: we
only referenced constants, and had to create them upfront.

Fixes #4887
Signed-off-by: Nathan Gauër <brioche@google.com>
2022-08-16 16:05:47 +02:00
Cassandra Beckley
3a8a961cff
Fix array copy propagation (#4890)
Array copy propagation was interpreting OpEntryPoint as a store
2022-08-11 09:59:37 -07:00
Steven Perron
7c5b17d379
Update passes to handle function declarations (#4599)
Spirv-opt has not had to handle module with function declarations.  This
lead many passes to assume that every function has a body.  This is not
always true.  This commit will modify a number of passes to handle
function declarations.

Fixes https://github.com/KhronosGroup/SPIRV-Tools/issues/4443
2021-10-28 11:54:37 -04:00
Jaebaek Seo
6a4da9da42
Debug info preservation in copy-prop-array pass (#3444)
When the pass replaces the local variable `OpVariable` ids to their
corresponding pointers, we have to update operands of DebugValue or
DebugDeclare instructions.
2020-07-06 13:48:12 -04:00
Steven Perron
248c80b049
Handle OpConstantNull in copy-prop-arrays. (#2870)
Many of the places in copy propagate arrays assumes that integer constant will be defined by an OpConstant instruction.  That is not always true.  We fix these spots by allowing for an OpConstantNull.
2019-09-19 10:24:00 -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
715afb0cea
Add a nullptr check to array copy propagation. (#1987)
We are missing a check for a nullptr that is causing things to fail.

Added an extra test case, and fixed up others.

This is the fix for https://github.com/Microsoft/DirectXShaderCompiler/issues/1598.
2018-10-19 12:53:40 -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
Steven Perron
146eb3bdcf
Fix erroneous uses of the type manager in copy-prop-arrays. (#1942)
There are a few spots where copy propagate arrays is trying
to go from a Type to an id, but the type is not unique.  When generating
code this pass needs specific ids, otherwise we get type mismatches.
However, the ambigous types means we can sometimes get the wrong type
and generate invalid code.

That code has been rewritten to not rely on the type manager, and just
look at the instructions instead.

I have opened https://github.com/KhronosGroup/SPIRV-Tools/issues/1939 to
try to get a way to make this more robust.
2018-10-01 14:45:44 -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
dan sinclair
e6b953361d
Move the ir namespace to opt. (#1680)
This CL moves the files in opt/ to consistenly be under the opt::
namespace. This frees up the ir:: namespace so it can be used to make a
shared ir represenation.
2018-07-09 11:32:29 -04:00
Steven Perron
af430ec822 Add pass to fold a load feeding an extract.
We have already disabled common uniform elimination because it created
sequences of loads an entire uniform object, then we extract just a
single element.  This caused problems in some drivers, and is just
generally slow because it loads more memory than needed.

However, there are other way to get into this situation, so I've added
a pass that looks specifically for this pattern and removes it when only
a portion of the load is used.

Fixes #1547.
2018-05-14 15:40:34 -04:00
Steven Perron
742454968d OpName and decorations should not stop array copy prop. 2018-04-04 22:24:10 -04:00
Steven Perron
c33af63264 Teach array copy propagation about OpImageTexelPointer.
OpImageTexelPointer acts like a special kind of load.  It is not an
array load, but it also cannot be removed the same way a regular
load can.  The type of propagation that needs to be done is similar
to what we do for arrays, so I want to merge that code into that
optmization.

Contributers to #1445.
2018-04-04 13:42:51 -04:00
Steven Perron
cbceeceab4 In copy-prop-arrays, indentify copies via OpCompositeInsert
When the original code copies an entire array or struct one element at a
time, this turns into a series of OpCompositeInsert instruction followed
by a store of the whole array.  We currently miss opportunities in copy
propagate arrays because we do not recognize this as a copy.

This commit adds code to copy propagate arrays to identify this code
pattern.

Also updates the performance passed to run array copy propagation.
2018-03-29 09:39:55 -04:00
Steven Perron
d8ca09821d Handle non-constant accesses in memory objects (copy prop arrays)
The first implementation of MemroyObject, which is used in copy
propagate arrays, forced the access chain to be like the access chains
in OpCompositeExtract.  This excluded the possibility of the memory
object from representing an array element that was extracted with a
variable index.   Looking at the code, that restriction is not
neccessary.  I also see some opportunities for doing this in some real
shaders.

Contributes to #1430.
2018-03-28 20:23:47 -04:00
Steven Perron
5e07ab1358 Handle more cases in copy propagate arrays.
When we change the type of an object that gets stored, we do not want to
change the type of the memory location being stored to.  In order to
still be able to do the rewrite, we will decompose and rebuild the
object so it is the type that can be stored.

Fixes #1416.
2018-03-27 11:04:49 -04:00
Steven Perron
c4dc046399 Copy propagate arrays
The sprir-v generated from HLSL code contain many copyies of very large
arrays.  Not only are these time consumming, but they also cause
problems for drivers because they require too much space.

To work around this, we will implement an array copy propagation.  Note
that we will not implement a complete array data flow analysis in order
to implement this.  We will be looking for very simple cases:

1) The source must never be stored to.
2) The target must be stored to exactly once.
3) The store to the target must be a store to the entire array, and be a
copy of the entire source.
4) All loads of the target must be dominated by the store.

The hard part is keeping all of the types correct.  We do not want to
have to do too large a search to update everything, which may not be
possible, do we give up if we see any instruction that might be hard to
update.

Also in types.h, the element decorations are not stored in an std::map.
This change was done so the hashing algorithm for a Struct is
consistent.  With the std::unordered_map, the traversal order was
non-deterministic leading to the same type getting hashed to different
values.  See |Struct::GetExtraHashWords|.

Contributes to #1416.
2018-03-26 14:44:41 -04:00