Disable trivial weak_ptr test on ARM because it is not expected to work.

[oontvoo]

Summary

weak_ptr has two pointers (more than the 4 bytes limit), so it will not be returned in registers on ARM, even if it is trivial.
The test, therefore, will fail on ARM.

[ldionne]

In that case, shouldn't it be an error to try to make weak_ptr trivial on ARM? The current state of things appears to be that one will say "make weak_ptr trivial", and the library will silently fail to honour that request.

[oontvoo]

In D84200#2164127, @ldionne wrote:

In that case, shouldn't it be an error to try to make weak_ptr trivial on ARM? The current state of things appears to be that one will say "make weak_ptr trivial", and the library will silently fail to honour that request.

An error by the compiler? IIUC, the attribute is a "request". It can try to honour it but doesn't have to if it can't. (probably similar to how inline and a few other directives are treated)

Or are you suggesting we detect this in the __config and #error (or #warn )? Is it customary for libraries in libcxx to do that?

[ldionne]

In D84200#2164275, @oontvoo wrote:

In D84200#2164127, @ldionne wrote:

In that case, shouldn't it be an error to try to make weak_ptr trivial on ARM? The current state of things appears to be that one will say "make weak_ptr trivial", and the library will silently fail to honour that request.

An error by the compiler? IIUC, the attribute is a "request". It can try to honour it but doesn't have to if it can't. (probably similar to how inline and a few other directives are treated)

Or are you suggesting we detect this in the __config and #error (or #warn )? Is it customary for libraries in libcxx to do that?

My point is that someone compiling for ARM will define _LIBCPP_ABI_ENABLE_SHARED_PTR_TRIVIAL_ABI thinking they are getting trivial weak_ptr and shared_ptr passed in registers, but they won't (and silently so). That deception is what I think should be flagged explicitly through an error of some sort. Do you agree?

[oontvoo]

My point is that someone compiling for ARM will define _LIBCPP_ABI_ENABLE_SHARED_PTR_TRIVIAL_ABI thinking they are getting trivial weak_ptr and shared_ptr passed in registers, but they won't (and silently so). That deception is what I think should be flagged explicitly through an error of some sort. Do you agree?

I can agree that it would seem a bit misleading.

I'm concerned that if we try to get into this business of flagging any potential unsupported case, we'll open a can of worms. That is, the checking here, if we were to be complete, would have to say, if the size of the struct is larger than the limit defined by the target platform, then it's an error. But how do we know what the limit is? There's the theory and there's the practice. For eg, the ARMv7 specs say it reserves 4 registers for the return value, but in practice, it looks like it allows only one (hence the 4 bytes limit). All of these heuristics may turn into an ugly hack (especially if we want to put them in libcxx).

[ldionne]

In D84200#2164525, @oontvoo wrote:

My point is that someone compiling for ARM will define _LIBCPP_ABI_ENABLE_SHARED_PTR_TRIVIAL_ABI thinking they are getting trivial weak_ptr and shared_ptr passed in registers, but they won't (and silently so). That deception is what I think should be flagged explicitly through an error of some sort. Do you agree?

I can agree that it would seem a bit misleading.

I'm concerned that if we try to get into this business of flagging any potential unsupported case, we'll open a can of worms. That is, the checking here, if we were to be complete, would have to say, if the size of the struct is larger than the limit defined by the target platform, then it's an error. But how do we know what the limit is? There's the theory and there's the practice. For eg, the ARMv7 specs say it reserves 4 registers for the return value, but in practice, it looks like it allows only one (hence the 4 bytes limit). All of these heuristics may turn into an ugly hack (especially if we want to put them in libcxx).

Similarly, one could argue the same for unique_ptr for some unknown platform where even a pointer can't be passed by register. Ok, I guess I'm with you now.

[jyknight]

Right, the trivial_abi attribute doesn't guarantee that you pass an object in registers, it just gives permission for the implementation to treat the struct as-if it was a trivial struct. On ARM32, since the ABI specifies that trivial structs > 4 bytes are returned indirectly by passing a hidden pointer to a pre-allocated return struct, that's what you get.

[oontvoo]

(Closing this ... assuming issue has been resolved)

[oontvoo]

Would anyone mind Accepting this so I could clear this revision off my pending queue? Thanks!
(It's already committed so ... )

libcxx/test/libcxx/memory/trivial_abi/weak_ptr_ret.pass.cpp

int main(int, char**) {
   std::weak_ptr<Node> ret = make_val(sptr, &local_addr);
   assert(local_addr != nullptr);
 
   // Without trivial_abi, &ret == local_addr because the return value
   // is allocated here in main's stackframe.
   //
   // With trivial_abi, local_addr is the address of a local variable in
   // make_val, and hence different from &ret.
+#ifndef __arm__
+  // On ARM32, structs larger than 4 bytes cannot be returned in registers.
+  // Thus, weak_ptr will be passed indrectly even if it is trivial.
   assert((void*)&ret != local_addr);
-
+#endif
   return 0;
 }