[PATCH] D137107: Allow MS extension: support of constexpr with __declspec(dllimport).

[Zahira Ammarguellat via Phabricator via cfe-commits]

zahiraam added a comment.

In D137107#3897568 <https://reviews.llvm.org/D137107#3897568>, @mstorsjo wrote:

> In D137107#3897326 <https://reviews.llvm.org/D137107#3897326>, @rnk wrote:
>
>> Unless I'm missing something, I think Clang's behavior here is preferable to MSVC's. MSVC produces code which will not link. Clang turns the linker error into a compiler error, which is generally easier for the user to understand. To my knowledge, it is still true that there is no COFF relocation which can statically initialize a global variable with the address of a dllimported symbol.
>>
>> For the use case you are considering, how do things work out in practice? Is there some new import patching facility that I'm not aware of? Is the variable linked statically instead of dynamically, and then the linker relaxes the __imp references to direct references?
>
> The testcase that is shown here is peculiar in the sense that `constexpr int& val_ref = val;` is located within `main()`, so it isn't really constexpr (as far as I'd undestand it), but MSVC produces a load from `__imp_val`.

I think I can be convinced that generating a compiler error instead of a link error for the first use case is better, but about this use case (I am not sure I understand why you say “it isn’t a real constexpr”):

extern int __declspec(dllimport) next(int n);
int main () {

  extern int _declspec(dllimport) val;
  constexpr int& val_ref = val;
  int i = next(val_ref);
  return i;

}

With MSVC with or without constexpr will lead to the same generated symbol

012 00000000 UNDEF  notype       External     | __imp_?val@@3HA (__declspec(dllimport) int val)

and the same assembly (load __imp_?val) (is that why you made the comment above?). And the test case runs.

But that’s not the case for clang (without this patch).
With constexpr it’s failing with the error mentioned above.

Without  constexpr the symbol generated is

0E 00000000 UNDEF  notype       External     | __imp_?val@@3HA (__declspec(dllimport) int val)

and the assembly generates a ‘load __imp_? Val’. Same than MSVC, but the test case fails to run.  @rnk Shouldn't this run?

>> I am ready to be wrong here: if things have changed and my info is stale, we should definitely adapt.
>
> I'm not aware of anything that would have changed here...
>
> For the cases where this needs to be really statically initialized, the mingw pseudo relocations do fill that spot (which fakes it by initializing it very early on, from a linker-generated list of spots in the binary that needs patching). For non-constexpr cases in C++, Clang solves it by implicitly generating a static constructor which fetches it from the dllimported variable.




Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D137107/new/

https://reviews.llvm.org/D137107