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

russell.gallop created this revision. russell.gallop added reviewers: craig.topper, cfe-commits. russell.gallop added a project: clang. Some of these test cases were using a variable instead of a literal so were not generating the immediate form of the corresponding instruction. Repository: r

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

lebedev.ri added a comment. .. or? Is this fixing a current test failure? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62850/new/ https://reviews.llvm.org/D62850 ___ cfe-commits mailing list cfe-commi

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

russell.gallop added a comment. > .. 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 ac

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

probinson added a comment. Is the compiler missing a check that these parameters are immediates? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62850/new/ https://reviews.llvm.org/D62850 ___ cfe-commits

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

russell.gallop added a comment. > 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

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

lebedev.ri added a comment. 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

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

russell.gallop added a comment. > 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 =

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

craig.topper added a comment. We support non immediate on these because gcc does. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62850/new/ https://reviews.llvm.org/D62850 ___ cfe-commits mailing list c

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

russell.gallop added a comment. > 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

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

russell.gallop added a comment. > 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)? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

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

lebedev.ri added a comment. 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

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

russell.gallop updated this revision to Diff 203124. russell.gallop added a comment. Re-added test cases using variables and added comment. This now tests both formats. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62850/new/ https://reviews.llvm.org/D62850 Files: clang/test/CodeGen

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

craig.topper accepted this revision. craig.topper added a comment. This revision is now accepted and ready to land. LGTM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62850/new/ https://reviews.llvm.org/D62850 ___ cfe-commits mailing list c

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

This revision was automatically updated to reflect the committed changes. Closed by commit rL362786: [X86][test] Add test cases using immediates to builtins-x86.c (authored by russell_gallop, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to c