This is a proposal how we could extend global fadd atomic builtins support for gfx908

Add tests for gfx908 with unsupported instructions.

Put back Real Instructions under SubtargetPredicate associated with gfx generation.

Changes after review.

In D135110#3859844, @mstorsjo wrote:

I don't quite know why, but it seems like this new library breaks tests of llvm-config; if I start out with an empty build directory and run ninja check-llvm (or ninja llvm-test-depends), then this library doesn't get built, and llvm-config prints llvm-config: error: missing: $HOME/code/llvm-project/llvm/build/lib/libLLVMFrontendHLSL.a - and that breaks tools/llvm-config/booleans.test and tools/llvm-config/system-libs.test.

Closing this review. This issue will be fixed in review : https://reviews.llvm.org/D135744

This change is fixing my issue. Thanks !
I will close my review.

Sorry for not providing more details.
These tests may fail if zlib is installed on the system, but front end is explicitly disabling LLVM_ENABLE_ZLIB in cmake, then LIT tests will fail with information that LLVM_ENABLE_ZLIB has been disabled.

In D124232#3479532, @sebastian-ne wrote:

In D124232#3479333, @piotr wrote:

In D124232#3477435, @sebastian-ne wrote:

I wanted to add this combine before, but I don’t think there is a way to add d16 to an instruction without potentially breaking the code.
The reason is, when an image_sample has the d16 flag enabled, it will use f32→f16 truncation or i32→i16 truncation, depending on the texture format in the descriptor.

Combining image_sample+fptrunc to image_sample d16 works fine for float textures, but I assume we don’t know at compile time if a texture is an integer or float texture.
The application may interpret stored values as float and does an fptrunc, but the texture is actually defined as an integer texture, so the hardware uses an integer trunc instead, giving different results.

Are you aware of any examples that would fail with this patch? At least in Vulkan, the image format must match the sampled type.

I see, I didn’t know the type on the intrinsic always matches the type of the texture. Sorry for the noise.
It would be nice to have the same combine with integer textures.

Compare pointers of OldIntr and InstToRemove instead of using isIdenticalTo.

In last version (which was merged and reverted) I made a mistake at the end
of modifyIntrinsicCall function where I was doing operations on removed
instruction (InstToReplace).

This change was merged to main and later reverted because of memory-sanitizer failing tests. I'm re-opening this review to publish updated version of the change.

I don't have commit access, can anyone merge this patch for me, please?

Changes after review