Page MenuHomePhabricator

wristow (Warren Ristow)
User

Projects

User does not belong to any projects.

User Details

User Since
Jan 29 2015, 2:13 PM (393 w, 1 d)

Recent Activity

Tue, Jul 26

wristow accepted D130493: Disable stack-sizes section by default for PS4..
Tue, Jul 26, 11:31 AM · Restricted Project, Restricted Project

Mon, Jul 25

wristow accepted D130493: Disable stack-sizes section by default for PS4..
Mon, Jul 25, 10:54 AM · Restricted Project, Restricted Project
wristow accepted D130495: Have a simple StackSizesSection on PS4..
Mon, Jul 25, 10:53 AM · Restricted Project, Restricted Project
wristow added a comment to D130448: [Reassociate][NFC] Use an appropriate `dyn_cast` for `BinaryOperator`.

Thanks for the quick reviews!

Mon, Jul 25, 10:28 AM · Restricted Project, Restricted Project
wristow committed rG3bbd380a5b51: [Reassociate][NFC] Use an appropriate dyn_cast for BinaryOperator (authored by wristow).
[Reassociate][NFC] Use an appropriate dyn_cast for BinaryOperator
Mon, Jul 25, 10:26 AM · Restricted Project, Restricted Project
wristow closed D130448: [Reassociate][NFC] Use an appropriate `dyn_cast` for `BinaryOperator`.
Mon, Jul 25, 10:25 AM · Restricted Project, Restricted Project

Sun, Jul 24

wristow requested review of D130448: [Reassociate][NFC] Use an appropriate `dyn_cast` for `BinaryOperator`.
Sun, Jul 24, 6:42 PM · Restricted Project, Restricted Project
wristow added a comment to D130408: [Reassociate][NFC] Consistent checking if FastMathFlags are suitable.

Thanks Sanjay!

Sun, Jul 24, 5:48 PM · Restricted Project, Restricted Project
wristow committed rG3089b411a465: [Reassociate][NFC] Consistent checking for FastMathFlags suitability (authored by wristow).
[Reassociate][NFC] Consistent checking for FastMathFlags suitability
Sun, Jul 24, 5:47 PM · Restricted Project, Restricted Project
wristow closed D130408: [Reassociate][NFC] Consistent checking if FastMathFlags are suitable.
Sun, Jul 24, 5:46 PM · Restricted Project, Restricted Project

Fri, Jul 22

wristow requested review of D130408: [Reassociate][NFC] Consistent checking if FastMathFlags are suitable.
Fri, Jul 22, 7:39 PM · Restricted Project, Restricted Project

Fri, Jul 15

wristow committed rGc6507930493b: [Reassociate] Enable FP reassociation via 'reassoc' and 'nsz' (authored by wristow).
[Reassociate] Enable FP reassociation via 'reassoc' and 'nsz'
Fri, Jul 15, 11:46 AM · Restricted Project, Restricted Project
wristow closed D129523: [Reassociate] Enable FP reassociation via 'reassoc' and 'nsz'.
Fri, Jul 15, 11:45 AM · Restricted Project, Restricted Project
wristow added a comment to D129523: [Reassociate] Enable FP reassociation via 'reassoc' and 'nsz'.

This is a straightforward translation of the existing FMF checks, so LGTM.
I added some inline comments for potential cleanups that could be done as NFC commits either before or after.

Fri, Jul 15, 11:24 AM · Restricted Project, Restricted Project

Thu, Jul 14

wristow updated the diff for D129523: [Reassociate] Enable FP reassociation via 'reassoc' and 'nsz'.

Update to:

  1. Remove the cleanup that has been done separately (and committed as 230c8c56f21cfe4e23a24793f3137add1af1d1f0).
  2. Check only for reassoc and nsz (rather than isFast()).
Thu, Jul 14, 10:08 PM · Restricted Project, Restricted Project

Jul 14 2022

wristow closed D129612: [Reassociate] Cleanup minor missed optimizations.

Committed at 230c8c56f21cfe4e23a24793f3137add1af1d1f0.

Jul 14 2022, 8:33 AM · Restricted Project, Restricted Project
wristow committed rG230c8c56f21c: [Reassociate] Cleanup minor missed optimizations (authored by wristow).
[Reassociate] Cleanup minor missed optimizations
Jul 14 2022, 8:22 AM · Restricted Project, Restricted Project
wristow updated the diff for D129612: [Reassociate] Cleanup minor missed optimizations.

Added an integer version of a reassociation test, including a TODO of a possible further improvement.

Jul 14 2022, 7:48 AM · Restricted Project, Restricted Project
wristow added a comment to D129612: [Reassociate] Cleanup minor missed optimizations.

I'd prefer to have that test, so we have coverage for the integer path added with this patch. It's still an improvement even if it's not as big as the FP side.
Other than that, LGTM.

Jul 14 2022, 7:40 AM · Restricted Project, Restricted Project

Jul 13 2022

wristow updated the diff for D129612: [Reassociate] Cleanup minor missed optimizations.

Addressed review comments from @spatel , except for adding an integer test.

Jul 13 2022, 5:20 PM · Restricted Project, Restricted Project
wristow added a comment to D129612: [Reassociate] Cleanup minor missed optimizations.

Is it possible to create an integer (mul and sub) test that also has this problem?

Jul 13 2022, 11:15 AM · Restricted Project, Restricted Project
wristow added a comment to D129612: [Reassociate] Cleanup minor missed optimizations.

To add some context to make it easier to review, the approach taken here of adding a ReassociatePass::OrderedSet parameter (named ToRedo) to the function LinearizeExprTree , is the same technique already used in two other functions in "Reassociate.cpp": NegateValue and BreakUpSubtract.

Jul 13 2022, 10:13 AM · Restricted Project, Restricted Project

Jul 12 2022

wristow added inline comments to D129523: [Reassociate] Enable FP reassociation via 'reassoc' and 'nsz'.
Jul 12 2022, 10:27 PM · Restricted Project, Restricted Project
wristow added a comment to D129523: [Reassociate] Enable FP reassociation via 'reassoc' and 'nsz'.

I'll split this into two independent patches.

I've split off the miscellaneous cleanup into a separate patch: D129612. Once that wraps up, I'll update the patch here to just be the improved optimization with reassoc and nsz.

Jul 12 2022, 6:21 PM · Restricted Project, Restricted Project
wristow requested review of D129612: [Reassociate] Cleanup minor missed optimizations.
Jul 12 2022, 6:17 PM · Restricted Project, Restricted Project
wristow added a comment to D129523: [Reassociate] Enable FP reassociation via 'reassoc' and 'nsz'.

Thanks for the quick feedback!

Jul 12 2022, 11:33 AM · Restricted Project, Restricted Project

Jul 11 2022

wristow added a comment to D129523: [Reassociate] Enable FP reassociation via 'reassoc' and 'nsz'.

Assuming this approach is sensible, I'm raising the question of whether this patch should be split into two.

Jul 11 2022, 4:27 PM · Restricted Project, Restricted Project
wristow requested review of D129523: [Reassociate] Enable FP reassociation via 'reassoc' and 'nsz'.
Jul 11 2022, 4:14 PM · Restricted Project, Restricted Project

Apr 26 2022

wristow committed rG35e7b4f82cfa: [NFC] Fix argument types in doxygen comment (authored by wristow).
[NFC] Fix argument types in doxygen comment
Apr 26 2022, 4:32 PM · Restricted Project, Restricted Project
wristow added a comment to D124469: [NFC] Cleanup miscellaneous header items.

Thanks for the review @RKSimon !

Apr 26 2022, 2:41 PM · Restricted Project, Restricted Project
wristow committed rGdf08b3493869: [NFC] Cleanup miscellaneous header items (authored by wristow).
[NFC] Cleanup miscellaneous header items
Apr 26 2022, 2:38 PM · Restricted Project, Restricted Project
wristow closed D124469: [NFC] Cleanup miscellaneous header items.
Apr 26 2022, 2:38 PM · Restricted Project, Restricted Project
wristow added a reviewer for D124469: [NFC] Cleanup miscellaneous header items: echristo.
Apr 26 2022, 12:45 PM · Restricted Project, Restricted Project
wristow requested review of D124469: [NFC] Cleanup miscellaneous header items.
Apr 26 2022, 11:30 AM · Restricted Project, Restricted Project

Apr 22 2021

wristow accepted D101104: [X86][AVX] foldShuffleOfHorizOp - don't attempt to handle 256-bit X86ISD::VBROADCAST (PR49971).

Clear simple, safe fix. And I've verified it fixes the problem we ran into downstream.
LGTM

Apr 22 2021, 3:57 PM · Restricted Project

Mar 31 2021

wristow accepted D93203: [PS4] handle dllimport/export w.r.t vtables/rtti.

LGTM!

Mar 31 2021, 11:58 AM · Restricted Project

Dec 8 2020

wristow added a comment to D67429: Improve code generation for thread_local variables:.

This commit appears to be the root cause of a run-time crash related to the interaction of global initializers and the wrapper functions to access thread_local variables -- reported as PR48030.

Dec 8 2020, 11:02 AM · Restricted Project, Restricted Project

Nov 18 2020

wristow accepted D91709: [X86][AVX] Only share broadcasts of different widths from the same SDValue of the same SDNode (PR48215).

LGTM

Nov 18 2020, 9:55 AM · Restricted Project

Sep 2 2020

wristow added a comment to D86907: Fix for PR46384. Failure on weak dllimport..

Tweaked the test to specifically look for the error of interest

Sep 2 2020, 5:29 PM · Restricted Project

Sep 1 2020

wristow accepted D86907: Fix for PR46384. Failure on weak dllimport..

Simplified the test case.

Sep 1 2020, 6:46 PM · Restricted Project
wristow added inline comments to D86907: Fix for PR46384. Failure on weak dllimport..
Sep 1 2020, 1:44 PM · Restricted Project
wristow added inline comments to D86649: Fix for assertion failure on PR46865.
Sep 1 2020, 11:55 AM · Restricted Project
wristow added inline comments to D86907: Fix for PR46384. Failure on weak dllimport..
Sep 1 2020, 11:52 AM · Restricted Project

Jun 22 2020

wristow added inline comments to D80952: [FPEnv][Clang][Driver] Disable constrained floating point on targets lacking support..
Jun 22 2020, 12:54 PM · Restricted Project

Mar 11 2020

wristow added a comment to D72675: [Clang][Driver] Fix -ffast-math/-ffp-contract interaction.

Revisiting this patch. I think that before fixing the -ffp-contract=off problem I originally raised here, there are two questions that have come up in this discussion that we should first resolve. Specifically:

Mar 11 2020, 1:01 PM · Restricted Project

Feb 26 2020

wristow accepted D74851: [x86] use instruction-level fast-math-flags to drive MachineCombiner.

LGTM

Feb 26 2020, 7:10 PM · Restricted Project

Jan 27 2020

wristow added inline comments to D72675: [Clang][Driver] Fix -ffast-math/-ffp-contract interaction.
Jan 27 2020, 2:55 PM · Restricted Project
wristow updated the diff for D72675: [Clang][Driver] Fix -ffast-math/-ffp-contract interaction.

Used the clearer '!off && !on' (rather than '!(off || on)') in the checks.

Jan 27 2020, 2:52 PM · Restricted Project

Jan 26 2020

wristow added a comment to D72675: [Clang][Driver] Fix -ffast-math/-ffp-contract interaction.

About:

This is a bit of an oddity in our handling.

Yes it is!

This is certainly getting more complicated than I had originally expected. I feel there isn't a clear spec on what we want in terms of whether FMA should be enabled "automatically" at (for example) '-O2', and/or whether it should be enabled by -ffast-math. I'm very willing to make whatever change is needed here, to meet whatever spec we all ultimately agree on.

So I think this patch should be put on hold until we decide on these wider aspects.

Jan 26 2020, 7:14 PM · Restricted Project

Jan 24 2020

wristow added inline comments to D72675: [Clang][Driver] Fix -ffast-math/-ffp-contract interaction.
Jan 24 2020, 7:18 PM · Restricted Project
wristow updated the diff for D72675: [Clang][Driver] Fix -ffast-math/-ffp-contract interaction.

Update a comment to remove the discussion about enabling the __FAST_MATH__ preprocessor macro. The handling of the setting of __FAST_MATH__ is done in "clang/lib/Frontend/InitPreprocessor.cpp", and once we decide on the set of conditions that enable that definition, we can add an appropriate comment there.

Jan 24 2020, 3:23 PM · Restricted Project
wristow added a comment to D72675: [Clang][Driver] Fix -ffast-math/-ffp-contract interaction.
  1. Should we enable FMA "by default" at (for example) '-O2'?

We recently introduced a new option "-ffp-model=[precise|fast|strict], which is supposed to serve as an umbrella option for the most common combination of options. I think our default behavior should be equivalent to -ffp-model=precise, which enables contraction. It currently seems to enable -ffp-contract=fast in the precise model (https://godbolt.org/z/c6w8mJ). I'm not sure that's what it should be doing, but whatever the precise model does should be our default.

Jan 24 2020, 3:14 PM · Restricted Project
wristow added a comment to D72675: [Clang][Driver] Fix -ffast-math/-ffp-contract interaction.

A separate question is the interaction of -ffast-math with -ffp-contract=. Currently, there is no such interaction whatsoever in GCC: -ffast-math does not imply any particular -ffp-contract= setting, and vice versa the -ffp-contract= setting is not considered at all when defining __FAST_MATH__. This seems at least internally consistent.

Jan 24 2020, 12:36 PM · Restricted Project

Jan 17 2020

wristow updated the diff for D72675: [Clang][Driver] Fix -ffast-math/-ffp-contract interaction.

Updated patch to correct a comment and fix a typo.

Jan 17 2020, 5:33 PM · Restricted Project

Jan 16 2020

wristow added a comment to D72529: [SCCIterator] Fix use-after-free.

Jinx ;) This took quite some tracking down on our side - too bad we didn't wait a couple of weeks.

Jan 16 2020, 2:44 PM · Restricted Project
wristow updated the diff for D72675: [Clang][Driver] Fix -ffast-math/-ffp-contract interaction.

Changing this to address only the Clang driver aspect, as discussed in the comments. Will spin off a separate patch for the LLVM side.

Jan 16 2020, 1:26 PM · Restricted Project
wristow added a comment to D72675: [Clang][Driver] Fix -ffast-math/-ffp-contract interaction.

One commit for the clang changes should be ok; it's a very small diff. But I'm still not sure if the driver change induces frontend diffs that we should make visible via tests.

The only thing I can think of is that it changes whether/when __FAST_MATH__ is defined. But that'll be indirectly tested, via the updated tests in "Driver/fast-math.c", which will verify that "-ffast-math" is passed appropriately (and the __FAST_MATH__ dependency on "-ffast-math" is already tested in "Preprocessor/predefined-macros.c").

Jan 16 2020, 12:56 PM · Restricted Project
wristow added a comment to D72675: [Clang][Driver] Fix -ffast-math/-ffp-contract interaction.

This all sounds good to me.

Jan 16 2020, 11:29 AM · Restricted Project

Jan 15 2020

wristow added inline comments to D72675: [Clang][Driver] Fix -ffast-math/-ffp-contract interaction.
Jan 15 2020, 5:38 PM · Restricted Project

Jan 14 2020

wristow added a comment to D72550: [SCCIterator] Fix another potential use-after-free.

This is a follow up to https://reviews.llvm.org/D72529.

Jan 14 2020, 11:58 PM · Restricted Project
wristow added a comment to D72529: [SCCIterator] Fix use-after-free.

Independently, we came across this same problem, and I proposed a patch at D72469 (which has been committed) that is essentially the simple fix that is described above:

Jan 14 2020, 11:21 PM · Restricted Project
wristow updated the diff for D72675: [Clang][Driver] Fix -ffast-math/-ffp-contract interaction.

Addressed comments from @hfinkel .

Jan 14 2020, 4:55 PM · Restricted Project
wristow added a comment to D72675: [Clang][Driver] Fix -ffast-math/-ffp-contract interaction.

Thanks for the quick feedback @hfinkel

Jan 14 2020, 2:45 PM · Restricted Project
wristow added a comment to D72469: SCC: Allow ReplaceNode to safely support insertion.

Seems to me we've needed to do this sort of tweak before, so LGTM.
Because it depends on the memory-management behavior of the map, it would be tricky to test with IR input. Would it be feasible to do something in llvm/unittests/ADT/SCCIteratorTest.cpp that could trigger this reliably? Can be done as a follow-up, as I know the branch is coming soon.

Jan 14 2020, 10:44 AM · Restricted Project
wristow committed rGf7e9f4f4c502: SCC: Allow ReplaceNode to safely support insertion (authored by wristow).
SCC: Allow ReplaceNode to safely support insertion
Jan 14 2020, 10:33 AM
wristow closed D72469: SCC: Allow ReplaceNode to safely support insertion.
Jan 14 2020, 10:33 AM · Restricted Project

Jan 13 2020

wristow created D72675: [Clang][Driver] Fix -ffast-math/-ffp-contract interaction.
Jan 13 2020, 7:19 PM · Restricted Project

Jan 9 2020

wristow created D72469: SCC: Allow ReplaceNode to safely support insertion.
Jan 9 2020, 11:29 AM · Restricted Project

Dec 19 2019

wristow added a comment to D71718: [X86] Mark various pointer arguments in builtins as const.

Thanks for the quick review.

Dec 19 2019, 1:34 PM · Restricted Project
wristow committed rG7fcd9e3f7083: [X86] Mark various pointer arguments in builtins as const (authored by wristow).
[X86] Mark various pointer arguments in builtins as const
Dec 19 2019, 11:53 AM
wristow closed D71718: [X86] Mark various pointer arguments in builtins as const.
Dec 19 2019, 11:53 AM · Restricted Project
wristow created D71718: [X86] Mark various pointer arguments in builtins as const.
Dec 19 2019, 10:32 AM · Restricted Project

Dec 13 2019

wristow added a comment to D69732: [WIP][LTO] Apply SamplePGO pipeline tunes for ThinLTO pre-link to full LTO.

Given the performance improvements here, I'd like to develop this patch further.

In D69732#1784290, @ormris wrote:
Ping @tejohnson

Dec 13 2019, 6:28 PM · Restricted Project, Restricted Project

Dec 12 2019

wristow committed rGbcae3a77afd1: [PS4] Predefine the __SCE__ macro for the x86_64-scei-ps4 triple (authored by wristow).
[PS4] Predefine the __SCE__ macro for the x86_64-scei-ps4 triple
Dec 12 2019, 11:10 AM

Nov 4 2019

wristow added inline comments to D69732: [WIP][LTO] Apply SamplePGO pipeline tunes for ThinLTO pre-link to full LTO.
Nov 4 2019, 6:38 PM · Restricted Project, Restricted Project
wristow added a comment to D69732: [WIP][LTO] Apply SamplePGO pipeline tunes for ThinLTO pre-link to full LTO.

This probably needs to be taken over by someone who cares about full LTO performance

Nov 4 2019, 6:29 PM · Restricted Project, Restricted Project

Oct 4 2019

wristow committed rL373790: Request commit access for wristow.
Request commit access for wristow
Oct 4 2019, 2:21 PM

Jul 19 2019

wristow accepted D64780: Disallow most calling convention attributes on PS4..

LGTM aside from a nit. You should give other reviewers a chance to sign off in case they have additional comments.

Jul 19 2019, 11:14 AM · Restricted Project, Restricted Project

Jul 17 2019

wristow added reviewers for D64780: Disallow most calling convention attributes on PS4.: rnk, rjmccall.
Jul 17 2019, 10:21 AM · Restricted Project, Restricted Project

Jul 15 2019

wristow added a comment to D64672: [X86] Prevent passing vectors of __int128 as <X x i128> in llvm IR.

Do we need to keep the old behavior on platforms where clang is the de facto compiler?

Jul 15 2019, 11:06 AM · Restricted Project

Jun 17 2019

wristow committed rG6452bdd29b5a: [LV] Suppress vectorization in some nontemporal cases (authored by wristow).
[LV] Suppress vectorization in some nontemporal cases
Jun 17 2019, 10:18 AM
wristow committed rL363581: [LV] Suppress vectorization in some nontemporal cases.
[LV] Suppress vectorization in some nontemporal cases
Jun 17 2019, 10:17 AM
wristow closed D61764: [LV] Suppress vectorization in some nontemporal cases.
Jun 17 2019, 10:16 AM · Restricted Project

Jun 13 2019

wristow added inline comments to D61764: [LV] Suppress vectorization in some nontemporal cases.
Jun 13 2019, 7:37 PM · Restricted Project

Jun 12 2019

wristow updated the diff for D61764: [LV] Suppress vectorization in some nontemporal cases.

Updated patch to address comments from @RKSimon cand @fhahn -- primarily change the default so that nontemporal misaligned mem-ops are assumed to not exist.

Jun 12 2019, 1:46 AM · Restricted Project

Jun 11 2019

wristow added a comment to D61764: [LV] Suppress vectorization in some nontemporal cases.

Thanks @fhahn. I'll update the patch to address those comments.

Jun 11 2019, 2:25 PM · Restricted Project
wristow added a comment to D61764: [LV] Suppress vectorization in some nontemporal cases.

This patch was accepted by @andreadb about a week ago, but Andrea said he'd leave the final decision to @RKSimon. With Simon's comments from a week ago asking @fhahn if he had any preferences for keeping the current default as-is, or changing it to something that seems to reflect existing hardware better, I've been hesitant to actually commit it, even though it's marked as Accepted. With a week now gone by, I'll plan on committing this tomorrow, unless Simon or Florian raise any concerns.

Jun 11 2019, 11:46 AM · Restricted Project

Jun 4 2019

wristow added inline comments to D61764: [LV] Suppress vectorization in some nontemporal cases.
Jun 4 2019, 10:53 AM · Restricted Project

May 31 2019

wristow added inline comments to D61764: [LV] Suppress vectorization in some nontemporal cases.
May 31 2019, 9:25 AM · Restricted Project
wristow added a comment to D61764: [LV] Suppress vectorization in some nontemporal cases.

Would it be possible to add tests where non-temporal load/stores successfully vectorize?

Glad to see your comment about SSE4A supporting nt-stores at any alignment. With that, I can make an X86 test-case that does vectorize.

May 31 2019, 1:05 AM · Restricted Project
wristow updated the diff for D61764: [LV] Suppress vectorization in some nontemporal cases.

Updated the patch to allow arbitrary alignment of float and double nt-stores for SSE4A.

May 31 2019, 12:54 AM · Restricted Project

May 28 2019

wristow added inline comments to D61764: [LV] Suppress vectorization in some nontemporal cases.
May 28 2019, 4:49 PM · Restricted Project
wristow added a comment to D61764: [LV] Suppress vectorization in some nontemporal cases.

Would it be possible to add tests where non-temporal load/stores successfully vectorize?

May 28 2019, 11:16 AM · Restricted Project

May 16 2019

wristow added a comment to D61764: [LV] Suppress vectorization in some nontemporal cases.

ping

May 16 2019, 3:38 PM · Restricted Project

May 9 2019

wristow added a comment to D61764: [LV] Suppress vectorization in some nontemporal cases.

One point I want to explicitly raise, is that prior to this proposed change, the current vectorization implementation ignores the nontemporal hint when checking whether a memory op is vectorizable. So conceptually, it's as though the two new routines:
bool isLegalNTStore(Type *DataType, unsigned Alignment)
bool isLegalNTLoad(Type *DataType, unsigned Alignment)
return true for all types and alignments. With that in mind, I've made the (default) target-independent implementations of these functions return true -- it's only the overriding functions on X86 (which is the target PR40759 was reported against) that will ever return false here (so on non X86 targets, this would be an NFC commit). But in looking through the other backends, I believe that none of them support misaligned nontemporal mem ops. So possibly a better default would be to have these new functions return true only if the specified DataType is minimally aligned at Alignment.

May 9 2019, 3:27 PM · Restricted Project
wristow created D61764: [LV] Suppress vectorization in some nontemporal cases.
May 9 2019, 3:22 PM · Restricted Project

May 8 2019

wristow added a comment to D55232: [SCEV] Suppress hoisting insertion point of binops when unsafe.

lgtm

May 8 2019, 11:50 AM · Restricted Project
wristow committed rGd27b0c624722: [SCEV] Suppress hoisting insertion point of binops when unsafe (authored by wristow).
[SCEV] Suppress hoisting insertion point of binops when unsafe
May 8 2019, 11:48 AM
wristow committed rL360280: [SCEV] Suppress hoisting insertion point of binops when unsafe.
[SCEV] Suppress hoisting insertion point of binops when unsafe
May 8 2019, 11:48 AM
wristow closed D55232: [SCEV] Suppress hoisting insertion point of binops when unsafe.
May 8 2019, 11:48 AM · Restricted Project