RFR (M): 8188224: Generalize Atomic::load/store to use templates

Kim Barrett kim.barrett at oracle.com
Sun Nov 12 04:47:07 UTC 2017


> On Nov 10, 2017, at 10:30 PM, David Holmes <david.holmes at oracle.com> wrote:
> 
> Hi Mikael,
> 
> On 11/11/2017 10:59 AM, Michael Dardis wrote:
>> Hi David,
>> Unfortunately I'm using VS2017 for a couple of reasons (64bit compiler in community edition, improved UWP support). Everything else works fine with a couple of very minor tweaks, but I'm having real trouble trying to deal with the templates here and figure out what's missing.
> 
> I've cc'd our C++ guru Kim Barrett.

I saw this, but haven’t had a chance to seriously look at it yet.

One thing I did notice, though, is that all the OrderAccess definitions (as opposed to declarations) are in
orderAccess.inline.hpp.  That doesn’t appear to be a change from the pre-templatization; all the pre-template
definitions were in the inline.hpp file too.

However, the offending call to OrderAccess::release_store is in a .hpp file (classLoader.hpp), which only
includes orderAccess.hpp (and it shouldn’t include orderAccess.inline.hpp, because .hpp files are not
supposed to include .inline.hpp files).  Which means the call in classLoader.hpp might only see the
declaration, and not the definition.  That seems like a bug.

I haven’t thought this through carefully, but this might be a two-phase name lookup issue.  Note that
VS2017 is the first version of VS that has two-phase name lookup.  Other compilers (gcc & clang)
have had two-phase lookup for a (very!) long time.  Assuming this is the right area to be looking (and
I’m not sure of that yet), it could be that VS2017 is being more strict than those other compilers (that
aren’t complaining about this code but maybe should?), or that VS2017 has a bug in this new (to it)
area.

There is a pretty thorough discussion of the two-phase name lookup issue in VS here:
https://blogs.msdn.microsoft.com/vcblog/2017/09/11/two-phase-name-lookup-support-comes-to-msvc/
It also discusses how to obtain the old behavior, using /Zc:twoPhase-.  It would be interesting to know
whether that made any difference for this problem.  (I don’t have easy access to VS2017 or even have
any familiarity with using it, or Windows for that matter.)




More information about the hotspot-dev mailing list