Page MenuHomePhabricator

jfb (JF Bastien)
User

Projects

User does not belong to any projects.

User Details

User Since
Jan 27 2014, 9:36 AM (273 w, 2 d)

Recent Activity

Today

jfb committed rGfb742da34c13: posix_spawn should retry upon EINTR (authored by jfb).
posix_spawn should retry upon EINTR
Wed, Apr 24, 4:24 PM
jfb committed rL359152: posix_spawn should retry upon EINTR.
posix_spawn should retry upon EINTR
Wed, Apr 24, 4:22 PM
jfb closed D61096: posix_spawn should retry upon EINTR.
Wed, Apr 24, 4:22 PM · Restricted Project
jfb added a comment to D61096: posix_spawn should retry upon EINTR.

Hmm, we have this utility function RetryAfterSignal() but it doesn't have maxRetries.

http://llvm.org/doxygen/namespacellvm_1_1sys.html#aea8b04d954b2cebd8a26d5d712634312

Wed, Apr 24, 3:39 PM · Restricted Project
jfb added a comment to D61096: posix_spawn should retry upon EINTR.

Wed, Apr 24, 3:00 PM · Restricted Project
jfb created D61096: posix_spawn should retry upon EINTR.
Wed, Apr 24, 2:59 PM · Restricted Project
jfb committed rG46d67fa6c5f9: Revert "[llvm-objdump] errorToErrorCode+message -> toString" (authored by jfb).
Revert "[llvm-objdump] errorToErrorCode+message -> toString"
Wed, Apr 24, 9:50 AM
jfb committed rL359110: Revert "[llvm-objdump] errorToErrorCode+message -> toString".
Revert "[llvm-objdump] errorToErrorCode+message -> toString"
Wed, Apr 24, 9:50 AM

Fri, Apr 19

jfb added inline comments to D59380: Fold constant & invariant loads into uses over barrier instructions.
Fri, Apr 19, 12:17 PM · Restricted Project

Thu, Apr 11

jfb committed rGef202c308b5f: Variable auto-init: also auto-init alloca (authored by jfb).
Variable auto-init: also auto-init alloca
Thu, Apr 11, 5:10 PM
jfb committed rL358243: Variable auto-init: also auto-init alloca.
Variable auto-init: also auto-init alloca
Thu, Apr 11, 5:10 PM
jfb committed rC358243: Variable auto-init: also auto-init alloca.
Variable auto-init: also auto-init alloca
Thu, Apr 11, 5:10 PM
jfb closed D60548: Variable auto-init: also auto-init alloca.
Thu, Apr 11, 5:10 PM · Restricted Project
jfb added a comment to D60548: Variable auto-init: also auto-init alloca.

I got an lgtm from @pcc on IRC, so I'll commit this.

Thu, Apr 11, 5:10 PM · Restricted Project
jfb added inline comments to D60548: Variable auto-init: also auto-init alloca.
Thu, Apr 11, 3:29 PM · Restricted Project
jfb updated the diff for D60548: Variable auto-init: also auto-init alloca.
  • Change name, qualify declaration.
Thu, Apr 11, 2:36 PM · Restricted Project
jfb added inline comments to D60548: Variable auto-init: also auto-init alloca.
Thu, Apr 11, 2:36 PM · Restricted Project
jfb updated the diff for D60548: Variable auto-init: also auto-init alloca.
  • Move patternFor to a shared file as requested by @rjmccall
Thu, Apr 11, 1:20 PM · Restricted Project
jfb added inline comments to D60548: Variable auto-init: also auto-init alloca.
Thu, Apr 11, 1:20 PM · Restricted Project
jfb added a comment to D60548: Variable auto-init: also auto-init alloca.
In D60548#1463181, @jfb wrote:
In D60548#1462124, @jfb wrote:

One downside to alloca is that we can's use __attribute__((uninitialized)) because it's a builtin. Maybe we need a separate uninitialized alloca? What do you all think?

Actually I'm wondering if we should just implement:

#pragma clang attribute push (__attribute__((uninitialized)), apply_to = variable(unless(is_global)))

And lean on that for alloca, instead of having a new uninitialized alloca builtin. The pragma is useful for debugging regardless, so I think overall it's better.

Thu, Apr 11, 12:46 PM · Restricted Project
jfb added a comment to D60548: Variable auto-init: also auto-init alloca.
In D60548#1462124, @jfb wrote:

One downside to alloca is that we can's use __attribute__((uninitialized)) because it's a builtin. Maybe we need a separate uninitialized alloca? What do you all think?

Thu, Apr 11, 12:46 PM · Restricted Project

Wed, Apr 10

jfb committed rCRT358145: [NFC] Use clearer naming for local variables.
[NFC] Use clearer naming for local variables
Wed, Apr 10, 4:25 PM
jfb committed rG2f46de8c0b25: [NFC] Use clearer naming for local variables (authored by jfb).
[NFC] Use clearer naming for local variables
Wed, Apr 10, 4:22 PM
jfb committed rL358145: [NFC] Use clearer naming for local variables.
[NFC] Use clearer naming for local variables
Wed, Apr 10, 4:21 PM
jfb accepted D60298: Fix a hang when lowering __builtin_dynamic_object_size.
Wed, Apr 10, 4:17 PM · Restricted Project
jfb added a comment to D60548: Variable auto-init: also auto-init alloca.

One downside to alloca is that we can's use __attribute__((uninitialized)) because it's a builtin. Maybe we need a separate uninitialized alloca? What do you all think?

Wed, Apr 10, 3:58 PM · Restricted Project
jfb created D60548: Variable auto-init: also auto-init alloca.
Wed, Apr 10, 3:57 PM · Restricted Project
jfb added a comment to D60480: [WIP] integration of pstl into libc++.

Not sure what is up with phabricator, but it won't let me respond inline to EricWF, specifically -

__FIRST_MOVER_ADVANTAGE

libc++ and libstdc++ have different 'uglification' protocols. As I said to mclow early on in this process, If I'm the one that has to do the grunt work of *all* of the uglification (very much has been case, BTW), it's going to follow libstdc++ convention as much as is possible.

libc++ code should follow libc++ conventions.

Wed, Apr 10, 10:01 AM · Restricted Project

Mon, Apr 8

jfb added a comment to D60372: [gn] Support for building libcxxabi.

For those without context, it would be really helpful if every GN commit had a link to documentation about what GN is, and how LLVM supports it. The initial commit had a note, maybe the LLVM docs should have this?

Mon, Apr 8, 9:28 AM · Restricted Project
jfb added a comment to D60352: [builtins] Use single line C++/C99 comment style.

Thanks @echristo for pointing out "[RFC] compiler-rt builtins cleanup and refactoring". You do intend to modify this code, so a reformat ahead of time is then fine. Please link to the RFC in the commit message.

Mon, Apr 8, 9:22 AM · Restricted Project, Restricted Project
jfb added a comment to D60351: [builtins] Reformat builtins with clang-format.

Thanks @echristo for pointing out "[RFC] compiler-rt builtins cleanup and refactoring". You do intend to modify this code, so a reformat ahead of time is then fine. Please link to the RFC in the commit message.

Mon, Apr 8, 9:22 AM · Restricted Project, Restricted Project

Sun, Apr 7

jfb added a comment to D60351: [builtins] Reformat builtins with clang-format.
In D60351#1457455, @jfb wrote:

I'd much rather see format updates happen as the files are touched, instead of all-out like this, because it makes mergebots sad.

Huh, usually the guidance is "land reformat in a separate commit". And we've mass-reformated code that didn't follow LLVM style before, e.g. lldb. What's different here?

Sun, Apr 7, 1:54 PM · Restricted Project, Restricted Project

Sat, Apr 6

jfb added a reviewer for D60351: [builtins] Reformat builtins with clang-format: jfb.
Sat, Apr 6, 2:50 PM · Restricted Project, Restricted Project
jfb added a comment to D60351: [builtins] Reformat builtins with clang-format.

I'd much rather see format updates happen as the files are touched, instead of all-out like this, because it makes mergebots sad.

Sat, Apr 6, 2:41 PM · Restricted Project, Restricted Project
jfb added a comment to D60352: [builtins] Use single line C++/C99 comment style.

Why? This seems like needless churn and is kinda annoying for mergebots.

Sat, Apr 6, 2:38 PM · Restricted Project, Restricted Project

Mar 25 2019

jfb committed rGcefafc499930: Thread Safety: also look at ObjC methods (authored by jfb).
Thread Safety: also look at ObjC methods
Mar 25 2019, 1:07 PM
jfb committed rC356940: Thread Safety: also look at ObjC methods.
Thread Safety: also look at ObjC methods
Mar 25 2019, 1:07 PM
jfb committed rL356940: Thread Safety: also look at ObjC methods.
Thread Safety: also look at ObjC methods
Mar 25 2019, 1:07 PM
jfb closed D59523: Thread Safety: also look at ObjC methods.
Mar 25 2019, 1:07 PM · Restricted Project, Restricted Project

Mar 22 2019

jfb added a comment to D59523: Thread Safety: also look at ObjC methods.
In D59523#1440238, @jfb wrote:

It's less about the regressions and more about the kind of regressions we're testing against.

But the test also verifies that no diagnostics are omitted (// expected-no-diagnostics), so it isn't just a "this doesn't crash" test. Which is why I think it's a nice seed to build more systematic tests around it. I'd generally like to test this more systematically, but that isn't made easier if we're sprinkling the test cases over the code base.

No, I wrote the test purely to check that the crash is gone. These tests *require* a diagnostic check, or // expected-no-diagnostics, so I put the later.

They only require // expected-no-diagnostics because you pass in -verify on the RUN line. That isn't needed to test the crashing regression, which may be part of what's causing confusion.

Mar 22 2019, 3:55 PM · Restricted Project, Restricted Project
jfb updated the diff for D59523: Thread Safety: also look at ObjC methods.
  • No verify, no expected.
Mar 22 2019, 3:55 PM · Restricted Project, Restricted Project
jfb added a comment to D59523: Thread Safety: also look at ObjC methods.

It's less about the regressions and more about the kind of regressions we're testing against.

But the test also verifies that no diagnostics are omitted (// expected-no-diagnostics), so it isn't just a "this doesn't crash" test. Which is why I think it's a nice seed to build more systematic tests around it. I'd generally like to test this more systematically, but that isn't made easier if we're sprinkling the test cases over the code base.

Mar 22 2019, 3:15 PM · Restricted Project, Restricted Project
jfb added inline comments to D59523: Thread Safety: also look at ObjC methods.
Mar 22 2019, 1:42 PM · Restricted Project, Restricted Project
jfb updated the diff for D59523: Thread Safety: also look at ObjC methods.
  • Almost Never Auto.
Mar 22 2019, 1:42 PM · Restricted Project, Restricted Project
jfb added inline comments to D59523: Thread Safety: also look at ObjC methods.
Mar 22 2019, 10:24 AM · Restricted Project, Restricted Project
jfb added inline comments to D59523: Thread Safety: also look at ObjC methods.
Mar 22 2019, 10:07 AM · Restricted Project, Restricted Project

Mar 20 2019

jfb added inline comments to D59523: Thread Safety: also look at ObjC methods.
Mar 20 2019, 2:24 PM · Restricted Project, Restricted Project
jfb updated the diff for D59523: Thread Safety: also look at ObjC methods.
  • Simlify tests, share code
Mar 20 2019, 2:24 PM · Restricted Project, Restricted Project

Mar 19 2019

jfb updated the diff for D59523: Thread Safety: also look at ObjC methods.
  • Add test
Mar 19 2019, 5:01 PM · Restricted Project, Restricted Project
jfb added a comment to D59523: Thread Safety: also look at ObjC methods.

Actually I'm wrong, this repros properly, will send an update with test.

Mar 19 2019, 4:53 PM · Restricted Project, Restricted Project
jfb added a comment to D59523: Thread Safety: also look at ObjC methods.

I reduced the code I had to this:

struct __attribute__((capability("mutex"))) MyLock {
  void lock() __attribute__((acquire_capability())) {}
  void unlock() __attribute__((release_capability())) {}
};
Mar 19 2019, 4:53 PM · Restricted Project, Restricted Project
jfb updated the diff for D59523: Thread Safety: also look at ObjC methods.
  • Use suggested format.
Mar 19 2019, 4:44 PM · Restricted Project, Restricted Project
jfb added a comment to D28213: [Frontend] Correct values of ATOMIC_*_LOCK_FREE to match builtin.

It looks like it was reverted because it was breaking i386 BSD, where __GCC_ATOMIC_LLONG_LOCK_FREE is in fact supposed to be "1" (because cmpxchg8b isn't always available).

Mar 19 2019, 1:55 PM · Restricted Project
jfb added a comment to D28213: [Frontend] Correct values of ATOMIC_*_LOCK_FREE to match builtin.

Why was it reverted?

Mar 19 2019, 1:14 PM · Restricted Project
jfb committed rGd81df259b350: Fix char.traits.specializations.char8_t main return (authored by jfb).
Fix char.traits.specializations.char8_t main return
Mar 19 2019, 12:25 PM
jfb committed rL356504: Fix char.traits.specializations.char8_t main return.
Fix char.traits.specializations.char8_t main return
Mar 19 2019, 12:24 PM
jfb committed rCXX356504: Fix char.traits.specializations.char8_t main return.
Fix char.traits.specializations.char8_t main return
Mar 19 2019, 12:24 PM
jfb committed rGc3608fc0d6e9: Fix fenv.pass.cpp signature for main (authored by jfb).
Fix fenv.pass.cpp signature for main
Mar 19 2019, 11:27 AM
jfb committed rCXX356493: Fix fenv.pass.cpp signature for main.
Fix fenv.pass.cpp signature for main
Mar 19 2019, 11:27 AM
jfb committed rL356493: Fix fenv.pass.cpp signature for main.
Fix fenv.pass.cpp signature for main
Mar 19 2019, 11:27 AM
jfb added a comment to D59442: Enable Operand Reordering for Commutative Instructions in the FunctionComparator/MergeFunctions.

Cannot we canonicalize to have a smaller index as lhs ?

v[0] + v[1] - ok
v[1] + v[0] - swap to indexes

Mar 19 2019, 10:32 AM · Restricted Project
jfb updated subscribers of D59442: Enable Operand Reordering for Commutative Instructions in the FunctionComparator/MergeFunctions.
Mar 19 2019, 9:52 AM · Restricted Project
jfb added a comment to D59130: [llvm][Support] Provide interface to set thread priorities.

Realistically nobody reads the comments, so I'm OK not having them. I just want to know that @kadircet read the documentation for the platform-specific APIs being introduced, and has reasonable suspicion that no gotcha is being introduced.

Have you checked the latest changes in the patch? I've already added comments for those platform specific behaviors as documented in their relevant web pages(because you've requested before). And yes, I had already read the documentation beforehand, AFAICT no gotcha is being introduced.
Please feel free to either just read the parts of the documentation I included in comments or sources themselves in urls and notify if there are some gotchas in any platforms.

Mar 19 2019, 9:43 AM · Restricted Project
jfb updated subscribers of D59442: Enable Operand Reordering for Commutative Instructions in the FunctionComparator/MergeFunctions.

I was talking to @dexonsmith about this review, and we like where this will eventually get LLVM. However, what really makes me uneasy is that I'm not sure the example you're trying to optimize happens often enough to warrant the algorithmic complexity in trying to perform the matching. Here are few more thoughts:

Mar 19 2019, 9:25 AM · Restricted Project

Mar 18 2019

jfb added a comment to D59523: Thread Safety: also look at ObjC methods.

I don't understand this code at all, but what about BlockDecl?

Mar 18 2019, 5:17 PM · Restricted Project, Restricted Project
jfb added a comment to D59523: Thread Safety: also look at ObjC methods.

It seems weird how both do pretty similar things and e.g. duplicate getParamDecl.

Mar 18 2019, 5:02 PM · Restricted Project, Restricted Project
jfb created D59523: Thread Safety: also look at ObjC methods.
Mar 18 2019, 5:02 PM · Restricted Project, Restricted Project
jfb added a comment to D59130: [llvm][Support] Provide interface to set thread priorities.

Gotchas and symptoms are IMO dependent on the use-case. Since this is a generic API I'd say that documenting the behavior is the best we can do here.

So how about @kadircet just adds a note to the header that platform specific details might be important here (depending on the use-case) and that more information is in comments in relevant implementation? Is that what you mean @jfb?

Mar 18 2019, 12:15 PM · Restricted Project
jfb updated subscribers of D59494: AMDGPU: Add support for cross address space synchronization scopes (clang).
Mar 18 2019, 9:34 AM · Restricted Project
jfb added a comment to D59130: [llvm][Support] Provide interface to set thread priorities.
In D59130#1426403, @jfb wrote:
In D59130#1424778, @jfb wrote:

That doesn't answer my question though: if someone sets priority and on the platform the test witness acceptable forward progress, will it happen that another platform provide no forward progress for the same work. For example, some implementations don't let a background thread perform any I/O. In such a case, a mere logging statement would halt the background thread's progress.

Fair enough. But is this a theoretical concern for some abstract platform that might implement this function in future, or is it a concrete concern for one of the implementations in this review?

You're adding a "portable" abstraction to different platform APIs. The onus here is on you to make sure that the abstraction is portable

There is a reasonable limit to portability.

Mar 18 2019, 9:32 AM · Restricted Project
jfb added a comment to D59442: Enable Operand Reordering for Commutative Instructions in the FunctionComparator/MergeFunctions.

This example that I gave in my comment is a simplistic one that gets canonicalized.
However, this patch would handle more interesting examples as well. Such as the following one:

[...]

This makes the existing -mergefunc fail, but this patch fixes it.

I agree that canonicalization steps help a lot, but, of course, that I like the idea of having a more powerful function merger (as a proponent of one).
Mar 18 2019, 8:39 AM · Restricted Project

Mar 16 2019

jfb accepted D59446: CodeGen: Preserve packed attribute in constStructWithPadding..

Nice catch!

Mar 16 2019, 6:59 AM · Restricted Project, Restricted Project

Mar 15 2019

jfb added a comment to D59442: Enable Operand Reordering for Commutative Instructions in the FunctionComparator/MergeFunctions.

This seems like something that we should canonicalize in general, no? i.e. the function comparator should only compare the canonicalized form (which another pass would ensure), and never expect to see a non-canonical one. I'd much rather make sure something else canonicalizes this than try to merge all permutations.

Mar 15 2019, 5:21 PM · Restricted Project
jfb added a comment to D53000: [Support] exit with custom return code for SIGPIPE.
In D53000#1430211, @jfb wrote:

Hey Nick,

This change is causing problems in LLDB. We want to ignore SIGPIPE and this appears to be making that impossible, which doesn't sound fair for something like libSupport. I'm not at all familiar with this code, but can we at least check if the signal is supposed to be ignored here?

I think you'll want to set up something like InterruptFunction: by default we'll do what Nick added, but if you ask we can do something else for SIGPIPE.

Wouldn't that affect all signals? What about an atomic boolean to enable/disable this behavior? Like I said before, I personally don't think this it is expected from a library like Support to do this. It feels like this behavior should be opt-in.

Mar 15 2019, 2:21 PM · Restricted Project

Mar 14 2019

jfb added a comment to D53000: [Support] exit with custom return code for SIGPIPE.

Hey Nick,

This change is causing problems in LLDB. We want to ignore SIGPIPE and this appears to be making that impossible, which doesn't sound fair for something like libSupport. I'm not at all familiar with this code, but can we at least check if the signal is supposed to be ignored here?

Mar 14 2019, 4:28 PM · Restricted Project
jfb added a comment to D59281: [WebAssembly] "atomics" feature requires shared memory.

D) Application uses (non-existent in WebAssembly) signal handling support and atomics just enforce program order.
E) Application is single-threaded but does cooperative interleaving of functions (but doesn't call it threads).

Mar 14 2019, 2:38 PM · Restricted Project
jfb added a comment to D59375: Allow unordered loads to be considered invariant in CodeGen.

This part isn't something I've very familiar with, better if someone else reviews.

Mar 14 2019, 11:23 AM · Restricted Project
jfb added a comment to D59345: Allow code motion (and thus folding) for atomic (but unordered) memory operands.

I'm trying to figure out if this optimization is only correct for memory_order_java, or if it also applies to memory_order_relaxed. I can't see a case in your examples which wouldn't apply to memory_order_relaxed as well. @__simt__ WDYT?
I'm not asking you to do that work. It just seems like a valid follow-up.

I'm not sure. I believe that memory_order_relaxed maps to llvm's monotonic right? If so, then I *think* moving one is safe, but monotonic is just subtly different enough from unordered and not atomic that a more careful audit would be needed.

Mar 14 2019, 10:09 AM · Restricted Project
jfb requested changes to D59254: [RFC] Implementation of Clang randstruct.

I find it easier to understand the code by looking at the tests. When you add tests, please make sure you test for:

  • Alignment attribute
  • Packed attribute
  • No unique address attribute
  • Bit-fields
  • Zero-width bit-field
  • Zero or unsized array at end of struct
  • C++ inheritance with vtables
  • C++ virtual inheritance
  • Anonymous unions (and the anonymous struct extension)
  • Types with common initial sequence
Mar 14 2019, 9:23 AM · Restricted Project
jfb accepted D59345: Allow code motion (and thus folding) for atomic (but unordered) memory operands.

Overall this looks good.
Do you have tests with intervening fences (both atomic and thread), to make sure they block motion as expected?

Mar 14 2019, 8:36 AM · Restricted Project
jfb updated subscribers of D59345: Allow code motion (and thus folding) for atomic (but unordered) memory operands.
Mar 14 2019, 8:35 AM · Restricted Project

Mar 13 2019

Herald added a project to D55895: NFC: simplify Darwin environment handling: Restricted Project.

I think we might need to run it past the internal builds to see if anything breaks.

Mar 13 2019, 3:08 PM · Restricted Project
jfb accepted D59287: [X86] Only define __GCC_HAVE_SYNC_COMPARE_AND_SWAP_16 in 64-bit mode..

OK, that seems unfortunate but unlikely and consistency terrible with GCC. Let's do it.

Mar 13 2019, 2:12 PM · Restricted Project
jfb added a comment to D59307: Patch llvm bug 41033 concerning atomicity of statement expressions.
In D59307#1427644, @jfb wrote:

I think you also want to test C++ std::atomic ...

The bug described in https://bugs.llvm.org/show_bug.cgi?id=41033 doesn't occur using C++ atomic, I rewrote the test case this way, and it compiles without error. The "load" operation returns int since all the atomic operations occur under the covers using builtins. Does this convey the test you have in mind?

#include <atomic>
int fum(int y) {

std::atomic<int> x(1);
y = ({x.load();});

}

Mar 13 2019, 1:49 PM · Restricted Project
jfb added a comment to D59287: [X86] Only define __GCC_HAVE_SYNC_COMPARE_AND_SWAP_16 in 64-bit mode..

Is this ok with the backend fixed?

Mar 13 2019, 1:36 PM · Restricted Project
jfb updated subscribers of D58712: Changes for Installing SwiftPM for Apple Swift 5 Toolchain on PowerPC64LE.
Mar 13 2019, 11:57 AM · Restricted Project
jfb accepted D59308: [X86] Check for 64-bit mode in X86Subtarget::hasCmpxchg16b().

I think this is fine since it makes something less broken, and is consistent with your other patch on the clang side. Would be neat for someone else to double-check.

Mar 13 2019, 10:34 AM · Restricted Project
jfb updated subscribers of D59307: Patch llvm bug 41033 concerning atomicity of statement expressions.

From an offline discussion, I'm told that @jwakely uses this in GCC headers and expects some semantics from it? Jonathan, why?!?!

Mar 13 2019, 10:16 AM · Restricted Project
jfb added a comment to D59307: Patch llvm bug 41033 concerning atomicity of statement expressions.

Thinking some more, this is really a silly gotcha in code. We should probably follow what we're about to do with wg21.link/P1152 and ban chaining. Statement expressions should therefore not be allowed to return _Atomic, std::atomic, nor volatile.

Mar 13 2019, 10:08 AM · Restricted Project
jfb added a comment to D59287: [X86] Only define __GCC_HAVE_SYNC_COMPARE_AND_SWAP_16 in 64-bit mode..

Most if not all of the test cases in test/CodeGen/X86/atomic128.ll fail with a fatal error if you run it in 32-bit mode with -mattr=+cx16 Looks like the backend is also bad at checking 64 bit mode.

Mar 13 2019, 10:08 AM · Restricted Project
jfb updated subscribers of D59307: Patch llvm bug 41033 concerning atomicity of statement expressions.
Mar 13 2019, 9:57 AM · Restricted Project
jfb requested changes to D59307: Patch llvm bug 41033 concerning atomicity of statement expressions.

I think you also want to test C++ std::atomic as well as regular volatile.

Mar 13 2019, 9:57 AM · Restricted Project
jfb added a comment to D59287: [X86] Only define __GCC_HAVE_SYNC_COMPARE_AND_SWAP_16 in 64-bit mode..

Isn’t that setMaxAtomicWidth in the x86-64 derived class?

Mar 13 2019, 8:37 AM · Restricted Project
jfb added a comment to D59287: [X86] Only define __GCC_HAVE_SYNC_COMPARE_AND_SWAP_16 in 64-bit mode..

I don't think this is quite right: CX16 is literally "I have cmpxchg16b".

Mar 13 2019, 8:20 AM · Restricted Project

Mar 12 2019

jfb updated subscribers of D59130: [llvm][Support] Provide interface to set thread priorities.
In D59130#1424778, @jfb wrote:

That doesn't answer my question though: if someone sets priority and on the platform the test witness acceptable forward progress, will it happen that another platform provide no forward progress for the same work. For example, some implementations don't let a background thread perform any I/O. In such a case, a mere logging statement would halt the background thread's progress.

Fair enough. But is this a theoretical concern for some abstract platform that might implement this function in future, or is it a concrete concern for one of the implementations in this review?

Mar 12 2019, 9:44 AM · Restricted Project

Mar 11 2019

jfb accepted D59228: Fix typos in compiler-rt/lib/builtins/atomic.c.

How did this ever work?

Mar 11 2019, 1:32 PM · Restricted Project, Restricted Project
jfb updated subscribers of D59228: Fix typos in compiler-rt/lib/builtins/atomic.c.
Mar 11 2019, 1:32 PM · Restricted Project, Restricted Project
jfb updated subscribers of D59223: Objective-C++11: Support static_assert() in @interface/@implementation ivar lists and method declarations.
Mar 11 2019, 11:09 AM · Restricted Project
jfb added a comment to D59130: [llvm][Support] Provide interface to set thread priorities.
In D59130#1422858, @jfb wrote:

None of these check the return value of the priority calls. What's expected if it fails?

We can change it to return true on success and false on failure.

Do these actually give the same semantics? I'm worried that this will get used and on some platforms a thread might stop doing progress at all whereas on other platforms they'll still make progress. What's the intended use of this?

These are the functions with closest semantics in those three platforms. It tries to lower thread's priority such that it only runs if there's no non-background task running.

Mar 11 2019, 9:50 AM · Restricted Project

Mar 8 2019

jfb added inline comments to D59152: [libc++] Build <filesystem> support as part of the dylib.
Mar 8 2019, 1:45 PM · Restricted Project