Page MenuHomePhabricator

DavidTruby (David Truby)
User

Projects

User does not belong to any projects.

User Details

User Since
Oct 3 2018, 7:41 AM (103 w, 4 d)

Recent Activity

Thu, Sep 24

DavidTruby added inline comments to D88219: [flang][driver] Remove unnecessary includes in the unittest.
Thu, Sep 24, 5:19 AM · Restricted Project
DavidTruby committed rG956a84da0616: [flang] [OpenMP 4.5] Adding lit test cases for OpenMP Constructs. (authored by praveen).
[flang] [OpenMP 4.5] Adding lit test cases for OpenMP Constructs.
Thu, Sep 24, 5:09 AM
DavidTruby closed D87908: [flang] [OpenMP 4.5] Adding lit test cases for OpenMP Constructs..
Thu, Sep 24, 5:09 AM · Restricted Project, Restricted Project, Restricted Project
DavidTruby accepted D88219: [flang][driver] Remove unnecessary includes in the unittest.

LGTM, perhaps put NFC in the commit message?

Thu, Sep 24, 5:00 AM · Restricted Project

Wed, Sep 23

DavidTruby added a comment to D87908: [flang] [OpenMP 4.5] Adding lit test cases for OpenMP Constructs..

@kiranktp @DavidTruby Thanks for the review.

I do not have commit access . Can someone commit the changes on my behalf ?

Wed, Sep 23, 7:07 AM · Restricted Project, Restricted Project, Restricted Project

Tue, Sep 22

DavidTruby abandoned D85508: [flang] Add rudimentary empty function lowering to tco.

As discussed on flang-dev I am closing this to not block anyone else's progress on upstreaming.

Tue, Sep 22, 9:53 AM · Restricted Project
DavidTruby abandoned D85509: [flang] Add rudimentary bbc tool infrastructure..

As discussed on flang-dev I am closing this to not block anyone else's progress on upstreaming.

Tue, Sep 22, 9:53 AM · Restricted Project
DavidTruby abandoned D85510: [flang] Upstream lowering of empty functions and programs..

As discussed on flang-dev I am closing this to not block anyone else's progress on upstreaming.

Tue, Sep 22, 9:53 AM · Restricted Project
DavidTruby added inline comments to D87684: [mlir]Add Allocate Clause to OMP Parallel Operation Definition.
Tue, Sep 22, 6:47 AM · Restricted Project
DavidTruby committed rGbf202b8ce77c: [NFC][mlir] Remove llvm:: prefix from SmallVector in parallel pretty printer. (authored by DavidTruby).
[NFC][mlir] Remove llvm:: prefix from SmallVector in parallel pretty printer.
Tue, Sep 22, 6:46 AM
DavidTruby closed D88025: [NFC][mlir] Remove llvm:: prefix from SmallVector in parallel pretty printer..
Tue, Sep 22, 6:46 AM · Restricted Project
DavidTruby accepted D88001: [flang][msvc] Add explicit function template argument to applyLamda. NFC..

LGTM

Tue, Sep 22, 6:11 AM · Restricted Project
DavidTruby accepted D88052: [flang][msvc] Explicitly reference "this" inside closure. NFC..

LGTM

Tue, Sep 22, 6:11 AM · Restricted Project, Restricted Project
DavidTruby accepted D87961: [flang][msvc] Add explicit function template argument to applyFunction. NFC..

LGTM. Odd that msvc needs this clarification though.

Tue, Sep 22, 6:10 AM · Restricted Project, Restricted Project
DavidTruby accepted D87908: [flang] [OpenMP 4.5] Adding lit test cases for OpenMP Constructs..

LGTM!
@kiranktp I believe most of the semantic checks are still only targeting OpenMP 4.5, at least the ones that I implemented are as that was the target spec last time I checked. If this hasn't significantly changed then upgrading the tests to OpenMP 5 will cause failures.

Tue, Sep 22, 6:07 AM · Restricted Project, Restricted Project, Restricted Project

Mon, Sep 21

DavidTruby added a comment to D88025: [NFC][mlir] Remove llvm:: prefix from SmallVector in parallel pretty printer..

As requested on https://reviews.llvm.org/D87684

Mon, Sep 21, 8:23 AM · Restricted Project
DavidTruby requested review of D88025: [NFC][mlir] Remove llvm:: prefix from SmallVector in parallel pretty printer..
Mon, Sep 21, 8:23 AM · Restricted Project

Fri, Sep 18

DavidTruby added a comment to D86071: [MLIR][OpenMP] Add omp.do operation.

I have submitted an RFC on the openmp do loop design here: https://llvm.discourse.group/t/openmp-worksharing-loop-rfc/1815?u=davidtruby

Fri, Sep 18, 7:15 AM · Restricted Project, Restricted Project

Wed, Sep 16

DavidTruby accepted D87783: [Flang] Fixed installation permission of the "binary" flang.

LGTM

Wed, Sep 16, 2:55 PM · Restricted Project

Tue, Sep 15

DavidTruby added inline comments to D87684: [mlir]Add Allocate Clause to OMP Parallel Operation Definition.
Tue, Sep 15, 8:10 AM · Restricted Project
DavidTruby added inline comments to D87684: [mlir]Add Allocate Clause to OMP Parallel Operation Definition.
Tue, Sep 15, 8:00 AM · Restricted Project

Tue, Sep 8

DavidTruby accepted D87243: [cmake] Centralize LLVM_ENABLE_WARNINGS option.

LGTM and seems to work from the flang side. Wait for another review with more experience though please :)

Tue, Sep 8, 6:48 AM · Restricted Project, Restricted Project, Restricted Project
DavidTruby updated the diff for D86071: [MLIR][OpenMP] Add omp.do operation.

Capitalise all schedule clause values.

Tue, Sep 8, 6:47 AM · Restricted Project, Restricted Project
DavidTruby added a comment to D86071: [MLIR][OpenMP] Add omp.do operation.

@DavidTruby, In general, I was asking whether all clause enumeration values can begin with a Capital letter? Just the reserved keywords would look odd right?

Tue, Sep 8, 4:52 AM · Restricted Project, Restricted Project

Mon, Sep 7

DavidTruby accepted D87125: Update recipe for flang-aarch64 slaves.

LGTM

Mon, Sep 7, 6:38 AM
DavidTruby updated the diff for D86071: [MLIR][OpenMP] Add omp.do operation.

Address review comments.

Mon, Sep 7, 6:29 AM · Restricted Project, Restricted Project
DavidTruby added inline comments to D86071: [MLIR][OpenMP] Add omp.do operation.
Mon, Sep 7, 6:09 AM · Restricted Project, Restricted Project
DavidTruby reopened D86779: [MLIR][Shape] Merge `shape` to `std`/`scf` lowerings..

I've reverted this commit for now since it broke the flang and mlir buildbots:
http://lab.llvm.org:8011/builders/flang-aarch64-ubuntu
http://lab.llvm.org:8011/builders/mlir-nvidia
http://lab.llvm.org:8011/builders/mlir-windows

Mon, Sep 7, 5:38 AM · Restricted Project
DavidTruby added a reverting change for D86779: [MLIR][Shape] Merge `shape` to `std`/`scf` lowerings.: rG973800dc7cbe: Revert "[MLIR][Shape] Merge `shape` to `std`/`scf` lowerings.".
Mon, Sep 7, 5:38 AM · Restricted Project
DavidTruby added a reverting change for rG15acdd75439b: [MLIR][Shape] Merge `shape` to `std`/`scf` lowerings.: rG973800dc7cbe: Revert "[MLIR][Shape] Merge `shape` to `std`/`scf` lowerings.".
Mon, Sep 7, 5:38 AM
DavidTruby committed rG973800dc7cbe: Revert "[MLIR][Shape] Merge `shape` to `std`/`scf` lowerings." (authored by DavidTruby).
Revert "[MLIR][Shape] Merge `shape` to `std`/`scf` lowerings."
Mon, Sep 7, 5:38 AM
DavidTruby added a comment to D86779: [MLIR][Shape] Merge `shape` to `std`/`scf` lowerings..

This commit appears to remove the ShapeToSCF folder, but not the add_directory(ShapeToSCF) in the parent folder, meaning the CMake configure step can't complete without errors.

Mon, Sep 7, 5:22 AM · Restricted Project

Tue, Sep 1

DavidTruby resigned from D86172: [flang] Fix assert on constant folding of extended types.
Tue, Sep 1, 3:39 AM · Restricted Project, Restricted Project
DavidTruby added a comment to D86410: [flang][msvc] Avoid ambiguous overload from base class. NFC..

If this is now fixed in another way, can this review be closed?

Tue, Sep 1, 3:39 AM · Restricted Project, Restricted Project
DavidTruby added a comment to D86423: [flang][openacc] Lower loop with collapse clause.

Is it possible to add lit tests based on the generated mlir?

Tue, Sep 1, 3:37 AM · Restricted Project

Aug 18 2020

DavidTruby added a reviewer for D86071: [MLIR][OpenMP] Add omp.do operation: clementval.
Aug 18 2020, 6:33 AM · Restricted Project, Restricted Project
DavidTruby resigned from D86121: [mlir] Remove the use of "kinds" from Attributes and Types.
Aug 18 2020, 6:18 AM · Restricted Project
DavidTruby accepted D86132: [clang][driver]Add quotation mark in test/fortran.f95 to avoid false positive.

LGTM

Aug 18 2020, 6:16 AM · Restricted Project
DavidTruby requested changes to D86132: [clang][driver]Add quotation mark in test/fortran.f95 to avoid false positive.

On my clang line, when compiling a C file, this appears as "-cc1" not "cc1". I don't see a cc1as so I can't check that one but I assume it will be the same.

Aug 18 2020, 4:10 AM · Restricted Project

Aug 17 2020

DavidTruby committed rG3b338e53e956: [flang] Add preprocessor test for defines passed on the command line (authored by DavidTruby).
[flang] Add preprocessor test for defines passed on the command line
Aug 17 2020, 6:36 AM
DavidTruby closed D85967: [flang] Add preprocessor test for defines passed on the command line.
Aug 17 2020, 6:36 AM · Restricted Project
DavidTruby added a reviewer for D86071: [MLIR][OpenMP] Add omp.do operation: ftynse.
Aug 17 2020, 6:29 AM · Restricted Project, Restricted Project
DavidTruby added reviewers for D86071: [MLIR][OpenMP] Add omp.do operation: kiranchandramohan, mehdi_amini.
Aug 17 2020, 6:28 AM · Restricted Project, Restricted Project
DavidTruby requested review of D86071: [MLIR][OpenMP] Add omp.do operation.
Aug 17 2020, 6:25 AM · Restricted Project, Restricted Project

Aug 14 2020

DavidTruby added a comment to D85937: [flang][msvc] Split class declaration and constexpr variable definition. NFC..

You might have to use conditional preprocessing to make this workaround specific to MSVC.

The workaround is valid C++ and works fine with every compiler. So why add a #if maze?

Aug 14 2020, 2:35 PM · Restricted Project, Restricted Project
DavidTruby accepted D85646: [flang][msvc] Disambiguate injected class name..

Interesting that the compilers don't use the same lookup rule here. I wonder which is correct (my gut feeling is actually that MSVC is).

Aug 14 2020, 5:49 AM · Restricted Project, Restricted Project
DavidTruby resigned from D85622: Separate the Registration from Loading dialects in the Context.
Aug 14 2020, 5:46 AM · Restricted Project
DavidTruby abandoned D84022: [flang] Fix multi-config generator builds..

Fixed by D85078

Aug 14 2020, 5:46 AM · Restricted Project
DavidTruby accepted D85884: [Flang] Move markdown files(.MD) from documentation/ to docs/.

LGTM

Aug 14 2020, 5:45 AM · Restricted Project
DavidTruby accepted D85937: [flang][msvc] Split class declaration and constexpr variable definition. NFC..

As an aside would it be worth throwing a bug report at MSVC? I think the existing code is conformant so they might like to know about the bug.

Aug 14 2020, 5:44 AM · Restricted Project, Restricted Project
DavidTruby requested review of D85967: [flang] Add preprocessor test for defines passed on the command line.
Aug 14 2020, 5:26 AM · Restricted Project

Aug 13 2020

DavidTruby added a comment to D85862: [flang] Ensure Preprocessor::Define() saves macro names correctly.

LGTM! It would be great to have test so this doesn't regress

Aug 13 2020, 3:31 AM · Restricted Project, Restricted Project

Aug 12 2020

DavidTruby added a comment to D84334: [flang] Version information in flang/f18.

@DavidTruby so, how about replacing:

set(include ${FLANG_BINARY_DIR}/include/flang)

set(include ${FLANG_BINARY_DIR}/include/flang)
set(include ${CMAKE_CURRENT_BINARY_DIR})

with:

target_include_directories(f18
  ${CMAKE_CURRENT_BINARY_DIR}
  ${FLANG_BINARY_DIR}/include/flang 
)
Aug 12 2020, 5:13 PM · Restricted Project, Restricted Project
DavidTruby added inline comments to D84334: [flang] Version information in flang/f18.
Aug 12 2020, 5:09 PM · Restricted Project, Restricted Project
DavidTruby added a comment to D85828: [Flang] Move mark down documentation(md) files to reStructuredText(rst) file format..

I think md is the wave of the future. Other llvm subproject use md. Let's stick with what we have unless there's a compelling advantage to using rst or another format.

Aug 12 2020, 8:16 AM · Restricted Project
DavidTruby added inline comments to D85828: [Flang] Move mark down documentation(md) files to reStructuredText(rst) file format..
Aug 12 2020, 7:52 AM · Restricted Project
DavidTruby accepted D85816: Fix binaries directory for flang slaves gcc10 and clang10.

LGTM

Aug 12 2020, 6:01 AM
DavidTruby requested changes to D85816: Fix binaries directory for flang slaves gcc10 and clang10.
Aug 12 2020, 5:02 AM
DavidTruby accepted D85828: [Flang] Move mark down documentation(md) files to reStructuredText(rst) file format..

LGTM and they seem to render fine on github too, thanks for sharing that link so we can check that easily!

Aug 12 2020, 5:00 AM · Restricted Project
DavidTruby committed rG35bee3503f4c: [clang-tidy] prevent generated checks from triggering assertions on anonymous… (authored by bogser01).
[clang-tidy] prevent generated checks from triggering assertions on anonymous…
Aug 12 2020, 4:45 AM
DavidTruby closed D85218: In clang-tidy base checks prevent anonymous functions from triggering assertions.
Aug 12 2020, 4:45 AM · Restricted Project, Restricted Project

Aug 11 2020

DavidTruby accepted D85660: [flang][msvc] Use platform-independent primitives in temporary f18 driver..

LGTM! I wonder why we have all this duplicated code here though...

Aug 11 2020, 5:15 AM · Restricted Project, Restricted Project
DavidTruby accepted D85657: [flang][msvc] Remove default arguments for function specializations..

LGTM

Aug 11 2020, 5:12 AM · Restricted Project

Aug 10 2020

DavidTruby accepted D85218: In clang-tidy base checks prevent anonymous functions from triggering assertions.

LGTM - please wait for someone more familiar with clang-tidy to review as well

Aug 10 2020, 6:40 AM · Restricted Project, Restricted Project
DavidTruby added a reviewer for D85218: In clang-tidy base checks prevent anonymous functions from triggering assertions: DavidTruby.
Aug 10 2020, 6:39 AM · Restricted Project, Restricted Project

Aug 7 2020

DavidTruby added a comment to D85470: [Flang] Fix release blocker issue #46931 related to documentation. .

Oh I see, I didn't realise clang uses the default theme not the LLVM one! If lld also copies these files I think it is fine for us to do so as well.

Aug 7 2020, 8:00 AM · Restricted Project
DavidTruby added a comment to D85470: [Flang] Fix release blocker issue #46931 related to documentation. .

Sticking to llvm style sounds good. But as soon as it is changed in llvm we won't match the llvm style any more. Does clang have copies like this too?

I believe clang uses some cmake infrastructure to get doxygen and sphinx to pick up the theming stuff from LLVM. I at least can't see copies in clang.

Aug 7 2020, 6:59 AM · Restricted Project
DavidTruby resigned from D85356: [mlir] Remove the need to define `kindof` on attribute and type classes..
Aug 7 2020, 6:52 AM · Restricted Project
DavidTruby added a comment to D85470: [Flang] Fix release blocker issue #46931 related to documentation. .

Thanks for adding all this infrastructure!
I don't see any CMake changes here though, where is the new flag added?

Aug 7 2020, 6:52 AM · Restricted Project
DavidTruby updated the summary of D85509: [flang] Add rudimentary bbc tool infrastructure..
Aug 7 2020, 2:21 AM · Restricted Project
DavidTruby updated the summary of D85510: [flang] Upstream lowering of empty functions and programs..
Aug 7 2020, 2:21 AM · Restricted Project
DavidTruby requested review of D85510: [flang] Upstream lowering of empty functions and programs..
Aug 7 2020, 2:19 AM · Restricted Project
DavidTruby requested review of D85509: [flang] Add rudimentary bbc tool infrastructure..
Aug 7 2020, 2:16 AM · Restricted Project
DavidTruby requested review of D85508: [flang] Add rudimentary empty function lowering to tco.
Aug 7 2020, 2:14 AM · Restricted Project

Aug 6 2020

DavidTruby added a comment to D85489: [flang][NFC] Reformat files with current clang-format.

Interesting, thanks for the info!
The changes look sensible to me so maybe it's a new pass clang-tidy has added.

Aug 6 2020, 5:44 PM · Restricted Project, Restricted Project
DavidTruby accepted D85489: [flang][NFC] Reformat files with current clang-format.

Out of interest what led to these changes? Were these files not clang formatted correctly before or has clang-format changed?

Aug 6 2020, 5:33 PM · Restricted Project, Restricted Project

Aug 5 2020

DavidTruby accepted D85212: [flang] Add parser support for OpenMP allocate clause.

LGTM

Aug 5 2020, 7:38 AM · Restricted Project, Restricted Project

Aug 4 2020

DavidTruby requested changes to D85212: [flang] Add parser support for OpenMP allocate clause.

LGTM other than formatting, could you run clang-format over this please? Thanks!

Aug 4 2020, 8:19 AM · Restricted Project, Restricted Project

Aug 3 2020

DavidTruby accepted D85078: [flang] Fix multi-config generator builds.

LGTM thanks!

Aug 3 2020, 6:04 AM · Restricted Project

Jul 31 2020

DavidTruby added a comment to D84022: [flang] Fix multi-config generator builds..

@tskeith could you explain your rationale for reverting this please? I don't think it's reasonable to expect all patches to be tested with out-of-tree configurations by people that don't perform them, and reverting here seems to raise that expectation.

Out-of-tree builds is a supported configuration, documented in the README. I don't think developers should be expected to test every possible build configuration, but if a change breaks a build and can't be fixed right away it should be reverted.

Jul 31 2020, 3:10 PM · Restricted Project
DavidTruby added a comment to D84022: [flang] Fix multi-config generator builds..

@Meinersbur CMAKE_CFG_INTDIR is being used here indirectly by LLVM_RUNTIME_OUTPUT_INTDIR, which references it. I've also added a comment about the flang.sh script (that previously was getting installed on both platforms) to say that we will now only install it on Linux. I did it this way as I couldn't find a neater way to make it executable on POSIX platforms in a custom command than using chmod.

Jul 31 2020, 2:21 PM · Restricted Project
DavidTruby added a comment to D84022: [flang] Fix multi-config generator builds..

Out-of-tree builds are four times faster than in-tree builds. Since I build flang every time I review a change, I do several builds a day (in addition to the builds I do for my own work). Thus, this change significantly degrades my productivity.

If the in-tree incremental builds are slow, that seems like a cmake bug; there shouldn't be any difference in performance between the two. Unless you're not rebuilding the LLVM changes each time, which in the general case won't work especially as we get closer integration between flang and LLVM.

Jul 31 2020, 9:02 AM · Restricted Project
DavidTruby added inline comments to D84347: [MLIR,OpenMP] Lowering of parallel operation: proc_bind clause 2/n.
Jul 31 2020, 6:28 AM · Restricted Project, Restricted Project, Restricted Project
DavidTruby resigned from D84481: [flang][openacc] Handle optional end directive in combined construct.
Jul 31 2020, 6:26 AM · Restricted Project, Restricted Project
DavidTruby accepted D84855: [flang] Make interactive behaviour more obvious.

LGTM after clang-format fixes

Jul 31 2020, 6:06 AM · Restricted Project
DavidTruby resigned from D84867: [flang] Add placeholder ReleaseNotes file.
Jul 31 2020, 6:06 AM · Restricted Project
DavidTruby requested changes to D85014: [Flang] Checks for constraint C7110-C7115..
Jul 31 2020, 6:04 AM · Restricted Project
DavidTruby resigned from D84956: [zorg] Add Flang and MLIR PowerPC buildbot on Red Hat.
Jul 31 2020, 5:51 AM
DavidTruby accepted D84965: [flang][OpenMP] Added initial support for lowering OpenMP parallel construct.

LGTM other than possibly adding a lit test if the right infrastructure is there

Jul 31 2020, 5:51 AM · Restricted Project
DavidTruby added a comment to D84965: [flang][OpenMP] Added initial support for lowering OpenMP parallel construct.

If this lowers to the OpenMP MLIR Dialect, would it be possible to have some lit codegen tests on the generated IR from a sample input file? Unless not all the necessary infrastructure is in yet for this.

Jul 31 2020, 5:48 AM · Restricted Project
DavidTruby resigned from D84864: [flang] Add release notes for LLVM 11 release.
Jul 31 2020, 5:45 AM · Restricted Project
DavidTruby added a comment to D84022: [flang] Fix multi-config generator builds..

Hi Pete, I don't do out of tree builds myself and they aren't defended by buildbots which would be why I didn't spot that this broke that configuration.
You could possibly add if statements around the changes to have the old code in an out-of-tree configuration, using the FLANG_STANDALONE_BUILD cmake variable. Obviously then multi-config builds won't work out-of-tree still, but it's possible nobody cares about that configuration.

Jul 31 2020, 5:41 AM · Restricted Project

Jul 30 2020

DavidTruby accepted D84856: [flang] Add details to --help screen on default behaviour.
Jul 30 2020, 6:23 AM · Restricted Project
DavidTruby accepted D84857: [flang] Add -h as a synonym for help.
Jul 30 2020, 6:13 AM · Restricted Project
DavidTruby committed rG332170356e35: [flang] Fix multi-config generator builds. (authored by DavidTruby).
[flang] Fix multi-config generator builds.
Jul 30 2020, 1:57 AM
DavidTruby closed D84022: [flang] Fix multi-config generator builds..
Jul 30 2020, 1:57 AM · Restricted Project

Jul 27 2020

DavidTruby updated the diff for D84022: [flang] Fix multi-config generator builds..

The previous change broke builds again, so I think we should take the other option and just remove the unnecessary variable here.

Jul 27 2020, 7:06 AM · Restricted Project
DavidTruby updated the diff for D84022: [flang] Fix multi-config generator builds..

Removed unnecessary change to flang/test/lit.cfg.py

Jul 27 2020, 5:40 AM · Restricted Project

Jul 24 2020

DavidTruby added a comment to D83397: [flang] Replace uses of _Complex with std::complex.

Thanks for the additional context; I checked returning std::complex on x86_64 and Aarch64 before my comment and it worked fine; I guess the calling convention for x86_32 differs here in a way I didn't appreciate.
At least on those two architectures, at each optimisation level the way the value is returned matches: at O0 and O1 both return in memory and at O2 and O3 both return in registers.

Jul 24 2020, 8:17 AM · Restricted Project, Restricted Project
DavidTruby committed rG4ef2e594d5be: [flang] Run non-gtest unit tests with lit. (authored by DavidTruby).
[flang] Run non-gtest unit tests with lit.
Jul 24 2020, 7:13 AM