Project: skia Issues People Development process History Sign in
New issue
Advanced search Search tips
Issue 2767 windows XP chrome bot fails on call to InterlockedIncrement64
Starred by 1 user Project Member Reported by halcanary@google.com, Jul 18 2014 Back to list
Status: WontFix
Owner:
Closed: Jun 27
Cc:
Area: ----
Priority: Icebox
Type: Defect



Sign in to add a comment
4:59 PM <•bungeman> dpranke: I've got an xp vm up with Skia, and have repro'ed the problem locally
5:00 PM <•bungeman> someone intended to use an intrinsic, but it got turned into a call
5:00 PM <•bungeman> which doesn't exist on xp
5:00 PM <•dpranke> aha
5:00 PM <halcanary_> InterlockedIncrement64
5:00 PM <•dpranke> woot?
5:01 PM <•dpranke> scottmg, sheriffs: ^^
5:01 PM <halcanary_> We put it in Skia commit 00a8fae, which is a week old.
5:01 PM <•bungeman> are the xp bots that far behind?
5:01 PM <•dpranke> no
5:01 PM <•bungeman> or were they out of commission for a bit?
5:02 PM <•dpranke> had you done a roll between then and now?
5:02 PM <•bungeman> yeah, several
5:02 PM <•dpranke> odd
5:02 PM <•dpranke> maybe something else changed that was also contributing?
5:02 PM <•bungeman> that change should have rolled in on Tuesday
5:03 PM <•dpranke> the xp bots didn't keel over until last night, and they all keeled over on builds that contained the skia roll we reverted
5:03 PM <•bungeman> possible that someone started using it, and before it was just being optimized away (just not being linked in)
5:03 PM <halcanary_> maybe we weren't calling that new code?
5:03 PM <•dpranke> yeah, maybe
 
Project Member Comment 1 by halcanary@google.com, Jul 18 2014
When this gets resolved, we can start rollign again.
Project Member Comment 2 by bungeman@google.com, Jul 18 2014
Cc: mtkl...@google.com
Labels: -Priority-Medium Priority-Critical OpSys-Windows
Project Member Comment 3 by bungeman@google.com, Jul 18 2014
I have an XP VM where one may (slowly) reproduce. Note that InterlockedIncrement64 is supported on Vista and later. The intrinsic version _InterlockedIncrement64 is available only when building for x64 (and IPF, but we don't really care I suppose).

So long as we need to support XP, I think this is a three parter, where we can use _InterlockedIncrement64 on x64, SkLazyPtr to try and get InterlockedIncrement64 from kernel32.dll on 32, and if that doesn't work, fall back to mutexes. Or something like that. We could also do some silly assembly for 32 bit calling lock CMPXCHG8B directly, and maybe investigate ExInterlockedCompareExchange64 which has been exported by the kernel since Win2000.
Project Member Comment 4 by mtkl...@google.com, Jul 19 2014
Building on CAS seems pretty reasonable to me.  That was (is?) our backup plan for 32-bit MIPS.
For now I have reverted 91bdbc (Use the GrCacheable ID to eliminate the need for notifications to GrGpuGL ... - https://codereview.chromium.org/376703009) which introduced the call to the 64b atomic_inc. We have rolled into Chromium successfully with that patch reverted (https://codereview.chromium.org/403293002).
Project Member Comment 6 by hcm@google.com, Aug 14 2014
Cc: hcm@google.com
Labels: -Priority-Critical Priority-High
Are we going after the changes Ben proposed in #3?  This one is marked critical, trying to gauge the priority of the work that remains post revert (or if it will be done in a different bug).  Step down by 1 for now...
Project Member Comment 7 by halcanary@google.com, Aug 14 2014
Cc: -halcanary@google.com
Project Member Comment 8 by bsalo...@google.com, Aug 14 2014
Labels: -Priority-High Priority-Medium
I think so but with the revert the pri is much lower than critical.
Project Member Comment 9 by bsalo...@google.com, Sep 30 2015
Labels: -Priority-Medium Priority-Icebox
Project Member Comment 10 by halcanary@google.com, Jun 27
Status: WontFix
Sign in to add a comment