User Details
- User Since
- Jan 15 2018, 8:31 AM (233 w, 1 d)
Tue, Jun 28
I will let others comment, but I think this is a perfectly reasonable alternative, and I much prefer using indentation over the delimiter-remark approach. LGTM, assuming nobody else objects
Thu, Jun 23
Fri, Jun 17
Thank you for the patch! I will leave it to others with more knowledge of the history of diagnostics to comment on whether the premise is acceptable, but I have a few technical comments
Thu, Jun 16
Tue, Jun 14
Jun 1 2022
May 13 2022
May 11 2022
May 2 2022
Even with newlines forced via extra remarks, I'm not a big fan of the "-----------------------" remark; it doesn't interact well with other random remarks in the output, for example when I enable all remarks using the pattern '.*' I see:
remark: foo.cl:27:0: AMDGPU DAG->DAG Pattern Instruction Selection: Function: test_kernel: MI Instruction count changed from 0 to 6; Delta: 6 remark: foo.cl:27:0: 0 stack bytes in function remark: foo.cl:42:0: AMDGPU DAG->DAG Pattern Instruction Selection: Function: test_func: MI Instruction count changed from 0 to 4; Delta: 4 remark: foo.cl:42:0: 0 stack bytes in function remark: foo.cl:42:0: SI insert wait instructions: Function: test_func: MI Instruction count changed from 4 to 5; Delta: 1 remark: foo.cl:8:0: AMDGPU DAG->DAG Pattern Instruction Selection: Function: empty_kernel: MI Instruction count changed from 0 to 1; Delta: 1 remark: foo.cl:8:0: 0 stack bytes in function remark: foo.cl:52:0: AMDGPU DAG->DAG Pattern Instruction Selection: Function: empty_func: MI Instruction count changed from 0 to 1; Delta: 1 remark: foo.cl:52:0: 0 stack bytes in function remark: foo.cl:52:0: SI insert wait instructions: Function: empty_func: MI Instruction count changed from 1 to 2; Delta: 1 remark: <unknown>:0:0: BasicBlock: : 2
Apr 26 2022
Apr 18 2022
Apr 12 2022
Rebase
Apr 5 2022
Mar 29 2022
Remove frontend changes, and just drop the diagnostic in the backend. Update
some comments and naming.
Mar 28 2022
LGTM
@yaxunl Does excluding device-libs via COV_None make sense?
Do not add the no-hostcall attribute for "COV_None" modules, to allow for
the OpenCL implementation of hostcall in the device-libs.
Mar 25 2022
Mar 24 2022
Eliminate diagnostic, and instead add function attribute in Clang for pre-COV_5
OpenCL codegen.
Mar 22 2022
Mar 18 2022
Mar 17 2022
For reference, the error I'm changing to a warning was added as part of https://reviews.llvm.org/D70038, and the issue that made me consider this approach appears when building OCL conformance tests at -O0
Mar 10 2022
Mar 9 2022
I noted some small nits, but otherwise LGTM. I won't accept, as there are others who should probably review.
Mar 8 2022
Thank you for following up! The change looks correct, but I have some nits about the general approach. Let me know what you think.
Feb 28 2022
LGTM, thank you!
Feb 22 2022
Jan 18 2022
LGTM, thank you!
Jan 7 2022
Implement explicit return version of llvm::visit
Dec 16 2021
No worries, I understand the patch is pretty big and the template code is (unfortunately) dense.
Rebase, ping
Dec 15 2021
Rebase
Dec 14 2021
Dec 13 2021
LGTM, with only typos and minor nits around wording.
I went ahead with just leaving DIArgList alone, let me know what you think
I'm pushing the (rough) patch which tries to make the switch from inheriting
DIArglist from MDNode to ReplaceableMetadataImpl.
Nov 29 2021
Nov 19 2021
Nov 18 2021
Nov 17 2021
Nov 16 2021
Add enforcement of the always-owned-by-MetadataAsValue property of DIArgList to
the metadata verifier. I'm not sure this is sufficient, but at least it is a
start.
Nov 15 2021
Nov 12 2021
Update DIArgList::handleChangedOperand to propagate in-place changes which
would require the DIArgList become distinct up to the owning MetadataAsValue.
Nov 9 2021
Ping, @dblaikie are you OK with the patch as-is? I think it is likely not worth the effort of tracking down what MSVC configurations will accept the constexpr version.
Nov 5 2021
Nov 4 2021
I believe the only pending issue is resolved, so I plan to land this tomorrow
if nobody has any objections.
Remove misleading comment headers, strip "CSR_" prefix from regmask identifiers.
Nov 3 2021
Sep 28 2021
Is it possible to use soft links instead of copies? It would still be good to clean up afterwards, but if the host won't allow that cleanup in some cases at least we aren't leaving a big file around.
Sep 16 2021
LGTM thank you!
Sep 2 2021
Sep 1 2021
Aug 31 2021
Aug 30 2021
Aug 20 2021
So, I tried to work out the behavior of the spec parser, and I think my understanding of empty is faulty. Compare http://ben-kiki.org/ypaste/data/26953/index.html (all content is non-space characters) with http://ben-kiki.org/ypaste/data/26954/index.html (the middle line content is a single space).
I'm not sure I'm the best person for the job, but I tried to give my thoughts.
Aug 17 2021
Rebase
Aug 11 2021
Eliminate the getDistinct method entirely from DIExpression and DIArgList. I
went about this by splitting up the DEFINE_MDNODE_* macros so each MDNode
class can use an appropriate version based on their distinct/uniqued
properties.
Aug 2 2021
Jul 30 2021
Jul 28 2021
LGTM, thank you for catching these!
Jul 26 2021
LGTM, as long as @MaskRay is satisfied with the changes around easy_install. If MacOS still uses it but we want to discourage it on Linux then I like the current wording.
I think the change itself LGTM, but I did a grep for GNUInstallDirs and it it only used in two other places currently. I suppose that means it is OK to use, but it does seem to imply there can be cases where we install to inconsistent locations. I don't know who to add to get more info on this, though?
LGTM, I see no reason why this shouldn't apply for all generators.