In D146426#4207118, @aaron.ballman wrote:This feels like it's heading in the wrong direction -- the AST should not have holes in it. An invalid type should be replaced by a valid type (after diagnosing the invalid type, of course) so that we can keep as much of the AST around as possible (for example, we typically stub in int and continue compilation, as in: https://godbolt.org/z/MvxjGovGh), which should then result in a non-null ParmVarDecl. This way, we don't need to sprinkle nullptr checks all over the compiler when inspecting a function's parameters.
- Queries
- All Stories
- Search
- Advanced Search
- Transactions
- Transaction Logs
Feed All Stories
All Stories
All Stories
Today
Today
ilya-biryukov added a comment to D146426: [Sema] Fix crash on __fp16 parameters in template instantiations.
mnadeem added inline comments to D146333: [Flang] Exit gracefully with a useful message when we fail to lookup a target.
yln added a reviewer for D146351: sanitizer_common: Use plain thread_local for __sancov_lowest_stack definition.: thetruestblue.
Hi, thanks Vitaly!
In D145881#4204872, @Mordante wrote:Thanks! LGTM! IIRC you have commit access right?
Changes aaron suggested. Chose zip_longest instead of enumerate.
Mon, Mar 20, 12:18 PM · Restricted Project
In D109958#4206644, @nikic wrote:FWIW, I am quite unhappy with the implementation quality of this pass, but I don't think I have the energy to deal with this. In the future, due diligence for pass enablement needs to include a review of the pass implementation by a domain expert, if this was not already done as part of the initial implementation. (Domain expert = SCEV reviewer in this context.)
This update reverts part of a previous update and removes _LIBCPP_ASAN_ANNOTATE_ONLY_LONG.
Has this patch been tested against Chromium? Has it been tested elsewhere?
The more testing it's undergone, the more confident I can be.
phosek added a comment to D146355: [compiler-rt] Allow finding LLVMConfig if CMAKE_FIND_ROOT_PATH_MODE_PACKAGE is set to ONLY.
I still don't understand the motivation behind this change. You already explicitly set CMAKE_FIND_ROOT_PATH_MODE_PACKAGE to ONLY for Android so why do we need to change the default?
Mon, Mar 20, 12:10 PM · Restricted Project
jsjodin added a comment to D144657: [mlir] Add alloca address space handling to the data layout subsystem.
In D144657#4206620, @ftynse wrote:Sorry for delay, already-approved patches don't show up in the review stream :(
scott.linder accepted D145558: [Assignment Tracking][NFC] Use BitVectors as masks for SmallVectors rather than using DenseMaps.
In D145558#4199730, @Orlando wrote:Thank you @scott.linder for taking a look at this.
It might be worth including the rationale in the description/commit message, like you've done in other similar changes. I assume it is a performance improvement?
Yep there's an average of around 0.25% reduction in instructions retired and a 1.1% max-rss for the CTMark suite in LTO-O3-g builds (I'll update the description).
I'm leaving a bunch of edit suggestions, but feel free to ignore them! I was just going through the exercise of writing one possible implementation to prove to myself it is an actual improvement, YMMV
I appreciate the in-depth review and the suggestions all look good to me!
The diff has become quite large now so I'll could look at trying to factor out some of the suggestions into NFC patches?
mnadeem committed rGf67b481098cc: [Flang] Exit gracefully with a useful message when we fail to lookup a target (authored by mnadeem).
[Flang] Exit gracefully with a useful message when we fail to lookup a target
ABataev added a comment to D144958: [SLP]Initial support for reshuffling of non-starting buildvector/gather nodes..
In D144958#4206872, @aeubanks wrote:a more succinct version: https://godbolt.org/z/7YTqP89Mv shows that slp-vectorizer introduces loads from poison
ABataev committed rG59ff9d377770: [SLP]Fix PR61554: use of missing vectorized value in buildvector nodes. (authored by ABataev).
[SLP]Fix PR61554: use of missing vectorized value in buildvector nodes.
erichkeane added a comment to D146426: [Sema] Fix crash on __fp16 parameters in template instantiations.
In D146426#4207118, @aaron.ballman wrote:This feels like it's heading in the wrong direction -- the AST should not have holes in it. An invalid type should be replaced by a valid type (after diagnosing the invalid type, of course) so that we can keep as much of the AST around as possible (for example, we typically stub in int and continue compilation, as in: https://godbolt.org/z/MvxjGovGh), which should then result in a non-null ParmVarDecl. This way, we don't need to sprinkle nullptr checks all over the compiler when inspecting a function's parameters.
aaron.ballman added inline comments to D141497: [clang][Interp] Record initialization via conditional operator.
Sorry @sebpop but I don't think it is part of ARM mapping symbols ABI standard. What type of compiler are you using when observing this? Is there any way to create test to reproduce it?
srhines accepted D146355: [compiler-rt] Allow finding LLVMConfig if CMAKE_FIND_ROOT_PATH_MODE_PACKAGE is set to ONLY.
LGTM, but @phosek should also be able to confirm this is ok.
Mon, Mar 20, 12:04 PM · Restricted Project
w2yehia added reviewers for D146431: [AIX][Driver] Implement -mxcoff-build-id option.: qiongsiwu, stephenpeckham.
Mon, Mar 20, 12:04 PM · Restricted Project
LGTM
add "<0xHEXSTRING>" to the option documentation.
Mon, Mar 20, 12:03 PM · Restricted Project
efriedma committed rG5452d8607185: [llvm-readobj] Pretty-print IMAGE_WEAK_EXTERN_ANTI_DEPENDENCY. (authored by efriedma).
[llvm-readobj] Pretty-print IMAGE_WEAK_EXTERN_ANTI_DEPENDENCY.
Updated patch to pass clang-format check.
mravishankar updated the diff for D146254: Changes to `SCFFuseProducerOfSliceResult` to also return the operations created during fusion..
Rebase
I agree with @Mordante
aaron.ballman added reviewers for D146426: [Sema] Fix crash on __fp16 parameters in template instantiations: erichkeane, aaron.ballman.
This feels like it's heading in the wrong direction -- the AST should not have holes in it. An invalid type should be replaced by a valid type (after diagnosing the invalid type, of course) so that we can keep as much of the AST around as possible (for example, we typically stub in int and continue compilation, as in: https://godbolt.org/z/MvxjGovGh), which should then result in a non-null ParmVarDecl. This way, we don't need to sprinkle nullptr checks all over the compiler when inspecting a function's parameters.
nickdesaulniers added reviewers for D146339: [StackProtector] attribute __stack_chk_fail as NoReturn: void, nikic.
nridge committed rG57bfe25574a0: [clangd] Remove reundant use of getSpellingLoc() (authored by nridge).
[clangd] Remove reundant use of getSpellingLoc()
move vfprintf_internal and file_writer to templates
nridge added a comment to D145843: [clangd] Add option to always insert headers with <> instead of "".
My understanding is that a more elaborate configuration scheme has been proposed in https://github.com/clangd/clangd/issues/1367, and the feedback there was (quoting Sam from this comment):
TIFitis committed rG2d373e4dc7e9: [MLIR][OpenMP] Added OMPIRBuilder support for Target Data directives (authored by TIFitis).
[MLIR][OpenMP] Added OMPIRBuilder support for Target Data directives
Mon, Mar 20, 11:49 AM · Restricted Project, Restricted Project, Restricted Project, Restricted Project
Mon, Mar 20, 11:48 AM · Restricted Project, Restricted Project, Restricted Project, Restricted Project, Restricted Project
Mon, Mar 20, 11:47 AM · Restricted Project, Restricted Project, Restricted Project, Restricted Project
More files
Mon, Mar 20, 11:46 AM · Restricted Project, Restricted Project, Restricted Project, Restricted Project
agozillon added a comment to D146063: [Flang][OpenMP][MLIR] Add lowering from parse tree to MLIR support for Declare Target for functions and subroutines.
I believe I understand your concerns.
Mon, Mar 20, 11:39 AM · Restricted Project
Update a few more files.
Mon, Mar 20, 11:39 AM · Restricted Project, Restricted Project, Restricted Project, Restricted Project
Update for 16.0.0 release
Mon, Mar 20, 11:38 AM · Restricted Project
Update RISCVISAInfo.* and RISCVTargetParser.*
Mon, Mar 20, 11:36 AM · Restricted Project, Restricted Project, Restricted Project, Restricted Project
kuhar committed rG92416b63a57b: [ADT] Work around `enumerate` compilation error with modules enabled (authored by kuhar).
[ADT] Work around `enumerate` compilation error with modules enabled
Mon, Mar 20, 11:33 AM · Restricted Project, Restricted Project, Restricted Project, Restricted Project
jdoerfert added inline comments to D146081: [OpenMP][libomptarget] Init device when printing device info.
There was objection against using /usr/modules on discourse. I assumed that libc++ owns /usr/include/c++ and hides everything under there.
jdoerfert added a comment to D146371: [Clang][OpenMP]Solved the the always truth condition in Arm64.
Can you please upload a patch that does not reformat the entire file?
Also, add a commit message explaining what this does.
tra published D146448: [CUDA] Update cached kernel handle when the function instance changes. for review.
aeubanks added reviewers for D146446: [AlwaysInliner] Make legacy pass like the new pass: asbirlea, mtrofin.
update
clementval requested review of D146447: [flang] Handle polymorphic entities with rank > 0 in entry statement.
reames committed rG272ebd6957ef: [LSR] Inline getAlternateIVEnd and simplify [nfc] (authored by reames).
[LSR] Inline getAlternateIVEnd and simplify [nfc]
ping @StephenFan
jdoerfert added a comment to D140722: [OpenMP] Prefix outlined and reduction func names with original func's name.
can we update some tests to see the results?
aprantl added a comment to D146340: [ADT] Work around `enumerate` compilation error with modules enabled.
Oh wait, I didn't realize you haven't landed it yet :-)
aprantl added a comment to D146340: [ADT] Work around `enumerate` compilation error with modules enabled.
Thanks, but this does not seem to have worked: https://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake/52610/consoleText
peter.smith added inline comments to D140202: [lld][ARM][2/3]Big Endian support - Word invariant support.
aemerson committed rG41e9c4b88c28: [NFC][Outliner] Delete default ctors for Candidate & OutlinedFunction. (authored by aemerson).
[NFC][Outliner] Delete default ctors for Candidate & OutlinedFunction.
I just went through the patch and it LGTM. The table of contents was really useful here.
jhuber6 committed rG6bd4d717d577: [libc] Add environment variables to GPU libc test for AMDGPU (authored by jhuber6).
[libc] Add environment variables to GPU libc test for AMDGPU
This is essentially a GPU only change. Stamping anyway.
jansvoboda11 committed rGd1e00b6f136e: [clang][deps] Only cache files with specific extension (authored by jansvoboda11).
[clang][deps] Only cache files with specific extension
jrbyrnes added inline comments to D142782: [AMDGPU] Add basic support for extended i8 perm matching.
Thanks @arsenm for taking another look.
This looks correct, although I think there are a few places where we can simplify the code to remove the explicit endianness.
vitalybuka accepted D146351: sanitizer_common: Use plain thread_local for __sancov_lowest_stack definition..
I guess Android was that main reason was older API levels why don't use thread_local, probably our buildbot is still on them.
Wide use of thread_local may simplify sanitizers.
AdvenamTacet added a comment to D145482: [ASan] Remove sanity checks during annotation of contiguous container.
@vitalybuka could you land that revision as I don't have commiter rights?
AdvenamTacet updated the summary of D145482: [ASan] Remove sanity checks during annotation of contiguous container.
In D146198#4205441, @pcwang-thead wrote:In D146198#4202168, @michaelmaitland wrote:If there are some microarchitectures that can't be modeled, just add a new subroutine to upstream if approved.
Does this mean that subtarget routines must be added to the RISCVScheduleV file since the following function needs to know about the custom subroutine to do its isa checks:
// Helper class for generating a list of resource cycles of different LMULs. class ResourceCycles<list<ResourceCycle> resourceCycles, string mx> {I am concerned that the RISCVScheduleV file will take on bloat due to holding subtarget related routines if this is the case.
Yes. So I posted this patch here just to discuss how we should handle this.
For example, solutions may be:
- Add routines to RISCVScheduleV.td just as what I have done.
- Extend TableGen to support pass functions:
// Supposes that we have a Function class to present a function object that its parameters are function parameters. class TargetSubroutine<int base, string mx> : Function; // Then. Supposes that we have a new bang operator to apply this function to input parameters and the result is `ret`. class ResourceCycles<list<TargetSubroutine> subroutines, int base, string mx> { list<int> value = !foreach(subroutine, subroutines, !apply(subroutine, base, mx) ); } // In SchedXXX.td, we can define our own routines. class Multiplier<int base, string mx>:TargetSubroutine { // We return an int value calculated from mx. int ret = !mul(base, multiplier<mx>.value); }
- Some templates are flexible to specify cycles according to LMULs (I haven't figured out one...).
Ilyas Mustafazade <il.mystafa@gmail.com>
Thank you.