armel: regression in clang-14 on `invalid output constraint '+g,w' in asm`

[sylvestre]

template <typename Packet> void psincos_float(Packet &_x) {
  __asm__("" : "+g,w"(_x));
}

$ clang-13 foo.cpp works

$ clang-{14,15} fails with:

foo.cpp:3:16: error: invalid output constraint '+g,w' in asm
  __asm__("" : "+g,w"(_x));
               ^
1 error generated.

This code can be found in Eigen: https://gitlab.com/libeigen/eigen/-/commit/82d61af3a490154ad1c0ae2fe00c561095854897#859198abc81bc7db86d6add0a16b27c957a7358b_1066_1114

Initially reported here: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1017765

[llvmbot]

@llvm/issue-subscribers-backend-arm

[DavidSpickett]

I can reproduce in godbolt: https://godbolt.org/z/1h7jEnjsh (though what trunk means there not sure)

The constraint: + = read or write g = "Any register, memory or immediate integer operand is allowed, except for registers that are not general registers." (https://gcc.gnu.org/onlinedocs/gcc/Simple-Constraints.html#Simple-Constraints) w = "VFP floating-point registers d0-d31 and the appropriate subset d0-d15 based on command line options. Used for 64 bit values only. Not valid for Thumb1." (https://gcc.gnu.org/onlinedocs/gcc/Machine-Constraints.html#Machine-Constraints)

I'm assuming from context that the comma means "and". So this statement is used as a barrier that says "this barrier has outputs that change general registers, memory and floating point registers".

(confuses me a bit because how would you choose a register for such a contradictory constraint, but it doesn't have to make any choices if the asm is empty)

Anyway I'll bisect and see what changed.

[DavidSpickett]

Bisect points to https://reviews.llvm.org/D112135 / https://github.com/llvm/llvm-project/commit/cb9a0dc293cf4ca451d625c6a54e491d8c11e591.

Which was fixing https://bugs.llvm.org/show_bug.cgi?id=52230 which appears to be just that instead of clang erroring at parse time, it later failed with an assert.

[DavidSpickett]

Perhaps prior to this change, the constraint was passed down the backend which would have asserted but only if there were registers to be allocated for it. Since the asm was empty, it never attempted to do so and so never hit the assert.

And FWIW, latest gcc does allow the constraint.

[DavidSpickett]

Adding some kind of fpu makes it work https://godbolt.org/z/jjs1eczT6 and from the original bug report:

Architecture: armel (armv7l)

Not sure if that is soft float but for Linux I would assume it's hard float. Unless clang's default target there is soft float, which could explain the issue.

I think the question is, is it valid to specify constraints for things that cannot possibly be allocated on the target hardware - as long as we never try to allocate them. The change that caused this says no, it is not.

And perhaps why is clang on that machine defaulting to soft float (which seems bad in general for something like Eigen).

GCC accepts the constraint even when -msoft-float is used.

[jspricke]

the Debian armel port is soft float: https://wiki.debian.org/ArchitectureSpecificsMemo

[DavidSpickett]

Thanks, I had no idea it went back that far!

armel
armv5te since Debian 10 'buster' (gcc-8 8-20171215-1).

Before that, armv4te.

[DavidSpickett]

I have a clearer picture of the issue now. I was mistaken, the comma in the output constraint isn't "and" it's "or." https://gcc.gnu.org/onlinedocs/gcc/Multi-Alternative.html#Multi-Alternative.

If you only use w you'll see that GCC also errors when there is no FPU - https://godbolt.org/z/cr6zW85Kq. So checking for that is valid in isolation.

The difference is that GCC says "if any of these constraints are valid, allow it" and clang says "if all of these constraints are valid, allow it".

Sounds like clang should follow GCC on this one and accept "+g,w" because the "g" part is valid.

The method in question is TargetInfo::validateOutputConstraint which needs some way to accumulate the valid parts of the constraint instead of failing on the first invalid part. See https://github.com/llvm/llvm-project/blob/bf1fe2497d60efb6c9bb1ece0fce18fedfac7dd9/clang/lib/Basic/TargetInfo.cpp#L652-654.

Unfortunately I don't have time to fix this myself, anyone else feel free to have a go.