[X86] Fix builtins-x86.c test where it wasn't using immediates. NFC

[russell.gallop]

Summary

Some of these test cases were using a variable instead of a literal so were not generating the immediate form of the corresponding instruction.

[lebedev.ri]

.. or?
Is this fixing a current test failure?

[russell.gallop]

.. or?

I'm not sure what you mean.

Is this fixing a current test failure?

No. The test is not failing, but it is not doing what was intended as these builtins are for generating the immediate form of the corresponding instruction and they were generating actually generating a register form.

This test doesn't check the instructions generated are correct, but fixing that is a bigger task...

[probinson]

Is the compiler missing a check that these parameters are immediates?

[russell.gallop]

Is the compiler missing a check that these parameters are immediates?

I don't think that there can be a check, or this would have been noticed.

I don't know whether this is possible and/or desirable. Do users use one version of the builtin and want the compiler to decide whether to use the immediate form?

[lebedev.ri]

In D62850#1529081, @russell.gallop wrote:

.. or?

I'm not sure what you mean.

Is this fixing a current test failure?

No. The test is not failing, but it is not doing what was intended as these builtins are for generating the immediate form of the corresponding instruction and they were generating actually generating a register form.

Then the test should be failing? Or is the current form also legal?

This test doesn't check the instructions generated are correct, but fixing that is a bigger task...

In D62850#1529197, @russell.gallop wrote:

Is the compiler missing a check that these parameters are immediates?

I don't think that there can be a check, or this would have been noticed.

I don't know whether this is possible and/or desirable. Do users use one version of the builtin and want the compiler to decide whether to use the immediate form?

See https://llvm.org/docs/LangRef.html#parameter-attributes immarg

[russell.gallop]

Then the test should be failing? Or is the current form also legal?

Hmm, builtin_ia32_insertps128() errors if you pass a variable, but these don't (e.g.):
mytest.c:122:13: error: argument to 'builtin_ia32_insertps128' must be a constant integer

tmp_V4f = __builtin_ia32_insertps128(tmp_V4f, tmp_V4f, tmp_i);

I'll have a look and see if there is a reason why these don't fail in the same way (which would make the test fail in it's current form).

[craig.topper]

We support non immediate on these because gcc does.

[russell.gallop]

I'll have a look and see if there is a reason why these don't fail in the same way (which would make the test fail in it's current form).

These do not have the "I" prefix (// I -> Required to constant fold to an integer constant expression.) in BuiltinsX86.def which is probably why they don't error.

Checking against the LLVM side, there is a comment in IntrinsicsX86.td:

// Oddly these don't require an immediate due to a gcc compatibility issue.
def int_x86_mmx_pslli_w : GCCBuiltin<"__builtin_ia32_psllwi">,
            Intrinsic<[llvm_x86mmx_ty], [llvm_x86mmx_ty,
                       llvm_i32_ty], [IntrNoMem]>;

That comment was added recently by Craig in r355993. So it looks like they should work with either for compatibility.

[russell.gallop]

We support non immediate on these because gcc does.

Thanks. Your comment crossed in mid-air. Okay, so is this test worth changing, or should it have both versions (immediate and non-immediate)?

[lebedev.ri]

In D62850#1529588, @russell.gallop wrote:

We support non immediate on these because gcc does.

Thanks. Your comment crossed in mid-air. Okay, so is this test worth changing, or should it have both versions (immediate and non-immediate)?

Both, i guess, with a comment as to why they are the way they are.

[russell.gallop]

Re-added test cases using variables and added comment. This now tests both formats.

[craig.topper]

LGTM

clang/test/CodeGen/builtins-x86.c

#endif
   (void) __builtin_ia32_mfence();
 #ifndef OPENCL
   (void) _mm_mfence();
 #endif
   (void) __builtin_ia32_pause();
 #ifndef OPENCL
   (void) _mm_pause();
 #endif
-  tmp_V4s = __builtin_ia32_psllwi(tmp_V4s, tmp_i);
-  tmp_V2i = __builtin_ia32_pslldi(tmp_V2i, tmp_i);
-  tmp_V1LLi = __builtin_ia32_psllqi(tmp_V1LLi, tmp_i);
-  tmp_V4s = __builtin_ia32_psrawi(tmp_V4s, tmp_i);
-  tmp_V2i = __builtin_ia32_psradi(tmp_V2i, tmp_i);
-  tmp_V4s = __builtin_ia32_psrlwi(tmp_V4s, tmp_i);
-  tmp_V2i = __builtin_ia32_psrldi(tmp_V2i, tmp_i);
-  tmp_V1LLi = __builtin_ia32_psrlqi(tmp_V1LLi, tmp_i);
+  tmp_V4s = __builtin_ia32_psllwi(tmp_V4s, imm_i_0_8);
+  tmp_V2i = __builtin_ia32_pslldi(tmp_V2i, imm_i_0_8);
+  tmp_V1LLi = __builtin_ia32_psllqi(tmp_V1LLi, imm_i_0_8);
+  tmp_V4s = __builtin_ia32_psrawi(tmp_V4s, imm_i_0_8);
+  tmp_V2i = __builtin_ia32_psradi(tmp_V2i, imm_i_0_8);
+  tmp_V4s = __builtin_ia32_psrlwi(tmp_V4s, imm_i_0_8);
+  tmp_V2i = __builtin_ia32_psrldi(tmp_V2i, imm_i_0_8);
+  tmp_V1LLi = __builtin_ia32_psrlqi(tmp_V1LLi, imm_i_0_8);
   tmp_V1LLi = __builtin_ia32_pmuludq(tmp_V2i, tmp_V2i);
   tmp_V2LLi = __builtin_ia32_pmuludq128(tmp_V4i, tmp_V4i);
   tmp_V8s = __builtin_ia32_psraw128(tmp_V8s, tmp_V8s);
   tmp_V4i = __builtin_ia32_psrad128(tmp_V4i, tmp_V4i);
   tmp_V8s = __builtin_ia32_psrlw128(tmp_V8s, tmp_V8s);
   tmp_V4i = __builtin_ia32_psrld128(tmp_V4i, tmp_V4i);
   tmp_V2LLi = __builtin_ia32_psrlq128(tmp_V2LLi, tmp_V2LLi);
   tmp_V8s = __builtin_ia32_psllw128(tmp_V8s, tmp_V8s);
   tmp_V4i = __builtin_ia32_pslld128(tmp_V4i, tmp_V4i);
   tmp_V2LLi = __builtin_ia32_psllq128(tmp_V2LLi, tmp_V2LLi);
-  tmp_V8s = __builtin_ia32_psllwi128(tmp_V8s, tmp_i);
-  tmp_V4i = __builtin_ia32_pslldi128(tmp_V4i, tmp_i);
-  tmp_V2LLi = __builtin_ia32_psllqi128(tmp_V2LLi, tmp_i);
-  tmp_V8s = __builtin_ia32_psrlwi128(tmp_V8s, tmp_i);
-  tmp_V4i = __builtin_ia32_psrldi128(tmp_V4i, tmp_i);
-  tmp_V2LLi = __builtin_ia32_psrlqi128(tmp_V2LLi, tmp_i);
-  tmp_V8s = __builtin_ia32_psrawi128(tmp_V8s, tmp_i);
-  tmp_V4i = __builtin_ia32_psradi128(tmp_V4i, tmp_i);
+  tmp_V8s = __builtin_ia32_psllwi128(tmp_V8s, imm_i_0_8);
+  tmp_V4i = __builtin_ia32_pslldi128(tmp_V4i, imm_i_0_8);
+  tmp_V2LLi = __builtin_ia32_psllqi128(tmp_V2LLi, imm_i_0_8);
+  tmp_V8s = __builtin_ia32_psrlwi128(tmp_V8s, imm_i_0_8);
+  tmp_V4i = __builtin_ia32_psrldi128(tmp_V4i, imm_i_0_8);
+  tmp_V2LLi = __builtin_ia32_psrlqi128(tmp_V2LLi, imm_i_0_8);
+  tmp_V8s = __builtin_ia32_psrawi128(tmp_V8s, imm_i_0_8);
+  tmp_V4i = __builtin_ia32_psradi128(tmp_V4i, imm_i_0_8);
   tmp_V4i = __builtin_ia32_pmaddwd128(tmp_V8s, tmp_V8s);
   (void) __builtin_ia32_monitor(tmp_vp, tmp_Ui, tmp_Ui);
   (void) __builtin_ia32_mwait(tmp_Ui, tmp_Ui);
   tmp_V16c = __builtin_ia32_lddqu(tmp_cCp);
   tmp_V16c = __builtin_ia32_palignr128(tmp_V16c, tmp_V16c, imm_i);
   tmp_V8c = __builtin_ia32_palignr(tmp_V8c, tmp_V8c, imm_i);
 #ifdef USE_SSE4
   tmp_V16c = __builtin_ia32_pblendvb128(tmp_V16c, tmp_V16c, tmp_V16c);