[PATCH] D24337: Fix the Thumb test for vfloat intrinsics

[Renato Golin via llvm-commits]

rengolin added a comment.

Hi Pablo,

The test match is an unfortunate side-effect of regular expressions and how the CHECK lines behave, sounds like a good fix, but you shouldn't delete the test, as James said.

Regarding the ".code 32", I agree this is a problem, but I'm unsure what's the solution. Right now, since the default is ARM, we only emit when it's not. This is an emulation of the original GNU behaviour and, unless we have a good reason not to, we should be following the same pattern, especially for assembly code, which is a bit irrelevant in the LLVM world.

We also have that problem for all other directives that leak meaning (.align, .arch, .cpu, .fpu, .arch_extension, etc). But adding all of them to the beginning of each function is not the way to go. We're discussing no the side a way to add push/pop to those directives, at least supported in the LLVM assembler, and this could help this case.

Is this change at all related to the tests? If not, I suggest you refrain from submitting it until we can form a consensus. It may look like a simple matter, but backwards compatibility in this case can lead to invisible code generation failures.

cheers,
--renato


https://reviews.llvm.org/D24337