Page MenuHomePhabricator

appcs (Anmol P. Paralkar)
User

Projects

User does not belong to any projects.

User Details

User Since
Dec 14 2016, 10:50 AM (310 w, 5 d)

Recent Activity

Fri, Nov 4

appcs updated subscribers of D136047: [DAGCombiner] Option --combiner-select-seq.
Fri, Nov 4, 4:05 PM · Restricted Project, Restricted Project
appcs added reviewers for D136047: [DAGCombiner] Option --combiner-select-seq: RKSimon, spatel.
Fri, Nov 4, 4:04 PM · Restricted Project, Restricted Project
appcs updated the diff for D136047: [DAGCombiner] Option --combiner-select-seq.

Added a cost-benefit option: --combiner-select-seq-min-cost-benefit
Added a check: <ConstantSDNode>.getAPIntValue().getMinSignedBits() <= 64

Fri, Nov 4, 4:02 PM · Restricted Project, Restricted Project

Oct 27 2022

appcs added a reviewer for D136047: [DAGCombiner] Option --combiner-select-seq: kristof.beyls.
Oct 27 2022, 1:23 PM · Restricted Project, Restricted Project
appcs added a reviewer for D136047: [DAGCombiner] Option --combiner-select-seq: t.p.northover.
Oct 27 2022, 1:22 PM · Restricted Project, Restricted Project

Oct 16 2022

appcs added a reviewer for D136047: [DAGCombiner] Option --combiner-select-seq: Eugene.Zelenko.
Oct 16 2022, 2:55 PM · Restricted Project, Restricted Project
appcs added a reviewer for D136047: [DAGCombiner] Option --combiner-select-seq: fhahn.
Oct 16 2022, 2:51 PM · Restricted Project, Restricted Project
appcs added a reviewer for D136047: [DAGCombiner] Option --combiner-select-seq: vielmetti.
Oct 16 2022, 2:43 PM · Restricted Project, Restricted Project
appcs added a comment to D136047: [DAGCombiner] Option --combiner-select-seq.

For the test case from https://bugs.llvm.org/show_bug.cgi?id=51147 the (cross-compiling on RHEL8/x86_64) build times:
without -mllvm --combiner-select-seq: real 24m16.804s
with -mllvm --combiner-select-seq: real 0m0.897s
with about 25% code size reduction under the option.

Oct 16 2022, 2:41 PM · Restricted Project, Restricted Project
appcs updated the summary of D136047: [DAGCombiner] Option --combiner-select-seq.
Oct 16 2022, 1:25 PM · Restricted Project, Restricted Project
appcs added a comment to D136047: [DAGCombiner] Option --combiner-select-seq.

Thanks to the Works On Arm program https://www.arm.com/solutions/infrastructure/works-on-arm for providing an equinix-metal-arm64-cluster by Equinix https://www.equinix.com for developing this solution.

Oct 16 2022, 1:24 PM · Restricted Project, Restricted Project
appcs requested review of D136047: [DAGCombiner] Option --combiner-select-seq.
Oct 16 2022, 1:17 PM · Restricted Project, Restricted Project

Apr 1 2022

appcs added a comment to D122517: Rebase D23934: Add a -ffixed-date-time= flag that sets the initial value of __DATE__, __TIME__, __TIMESTAMP__.

Bazel uses -Wno-builtin-macro-redefined -D__DATE__="redacted" -D__TIMESTAMP__="redacted" -D__TIME__="redacted".
For reproducibility purposes, this is sufficient. I have two questions:

  • Do we need the arbitrary time feature?
  • Should an arbitrary string like "redacted" be supported?
Apr 1 2022, 6:10 PM · Restricted Project
appcs added a comment to D122517: Rebase D23934: Add a -ffixed-date-time= flag that sets the initial value of __DATE__, __TIME__, __TIMESTAMP__.

(From D121040) Please let me know about the following and I will incorporate your feedback and submit subsequent patches for addressing these questions:

a. Disallow the simultaneous use of -Wdate-time/-Wpch-date-time and -ffixed-date-time=<timestamp>?
b. Disallow -ffixed-date-time=<timestamp>:

0. if PCH's are used
1. in PCH compilation

Also, (just as -Wdate-time warns on TIMESTAMP use), do we (in a separate patch), extend the scope of -Wpch-date-time to cover the use of TIMESTAMP is in a PCH?

Thank you.

Apr 1 2022, 6:03 PM · Restricted Project
appcs added a comment to D122517: Rebase D23934: Add a -ffixed-date-time= flag that sets the initial value of __DATE__, __TIME__, __TIMESTAMP__.

Bazel uses -Wno-builtin-macro-redefined -D__DATE__="redacted" -D__TIMESTAMP__="redacted" -D__TIME__="redacted".
For reproducibility purposes, this is sufficient. I have two questions:

  • Do we need the arbitrary time feature?

If you are saying that defining DATE, TIMESTAMP, TIME to an arbitrary time value, then that is the definition of the problem that we are trying to solve; else, please clarify. Thanks.

  • Should an arbitrary string like "redacted" be supported?

Via -ffixed-date-time the value would have to be formatted e.g. 1969-12-31T23:59:59 (RFC3339 without fractional seconds and timezone); else, please clarify. Thanks.

Apr 1 2022, 2:28 PM · Restricted Project

Mar 29 2022

appcs added a reviewer for D122517: Rebase D23934: Add a -ffixed-date-time= flag that sets the initial value of __DATE__, __TIME__, __TIMESTAMP__: aaron.ballman.
Mar 29 2022, 5:41 PM · Restricted Project
appcs updated the diff for D122517: Rebase D23934: Add a -ffixed-date-time= flag that sets the initial value of __DATE__, __TIME__, __TIMESTAMP__.
  • Re-spun with std::get_time().
  • Expanded regression test with multiple error scenarios.
Mar 29 2022, 5:31 PM · Restricted Project

Mar 28 2022

appcs added inline comments to D122517: Rebase D23934: Add a -ffixed-date-time= flag that sets the initial value of __DATE__, __TIME__, __TIMESTAMP__.
Mar 28 2022, 3:08 PM · Restricted Project
appcs added a comment to D122517: Rebase D23934: Add a -ffixed-date-time= flag that sets the initial value of __DATE__, __TIME__, __TIMESTAMP__.

Bazel uses -Wno-builtin-macro-redefined -D__DATE__="redacted" -D__TIMESTAMP__="redacted" -D__TIME__="redacted".
For reproducibility purposes, this is sufficient. I have two questions:

  • Do we need the arbitrary time feature?
Mar 28 2022, 2:15 PM · Restricted Project

Mar 25 2022

appcs added a comment to D122517: Rebase D23934: Add a -ffixed-date-time= flag that sets the initial value of __DATE__, __TIME__, __TIMESTAMP__.

(From D121040) Please let me know about the following and I will incorporate your feedback and submit subsequent patches for addressing these questions:

Mar 25 2022, 4:36 PM · Restricted Project
appcs added a comment to D121040: Add a new preprocessor option `-fbuild-session-timestamp-date-time-macros`.

I will port https://reviews.llvm.org/D23934 to top of trunk, thank you.

Mar 25 2022, 4:33 PM · Restricted Project
appcs added a comment to D23934: Add a -ffixed-date-time= flag that sets the initial value of __DATE__, __TIME__, __TIMESTAMP__.

Hello, Will this feature be committed? Thanks!

Mar 25 2022, 4:31 PM · Restricted Project
appcs requested review of D122517: Rebase D23934: Add a -ffixed-date-time= flag that sets the initial value of __DATE__, __TIME__, __TIMESTAMP__.
Mar 25 2022, 4:29 PM · Restricted Project

Mar 7 2022

appcs added a comment to D121040: Add a new preprocessor option `-fbuild-session-timestamp-date-time-macros`.

...
Also, along the lines of similar questions in D23934, how does this affect -Wdate-time and -Wpch-date-time?

Mar 7 2022, 5:37 PM · Restricted Project
appcs added a comment to D121040: Add a new preprocessor option `-fbuild-session-timestamp-date-time-macros`.

Why isn't D23934 sufficient? What do these flags with very long names do that ffixed-date-time doesn't?

Mar 7 2022, 4:26 PM · Restricted Project

Mar 4 2022

appcs added a comment to D23934: Add a -ffixed-date-time= flag that sets the initial value of __DATE__, __TIME__, __TIMESTAMP__.

The original author appears to have given up on it. Someone will need to rebase and work on pushing it forward

Mar 4 2022, 8:17 PM · Restricted Project
appcs added reviewers for D121040: Add a new preprocessor option `-fbuild-session-timestamp-date-time-macros`: emaste, thakis, rsmith, bruno, ed, hfinkel, Alexander.
Mar 4 2022, 8:14 PM · Restricted Project
appcs requested review of D121040: Add a new preprocessor option `-fbuild-session-timestamp-date-time-macros`.
Mar 4 2022, 8:09 PM · Restricted Project

Feb 11 2022

appcs added a comment to D23934: Add a -ffixed-date-time= flag that sets the initial value of __DATE__, __TIME__, __TIMESTAMP__.

Hello, Will this feature be committed? Thanks!

Feb 11 2022, 7:49 AM · Restricted Project

Jul 16 2019

appcs added a reviewer for D64830: [Xtensa 4/10] Add basic *td files with Xtensa architecture description.: appcs.
Jul 16 2019, 4:04 PM · Restricted Project, Restricted Project
appcs accepted D64829: [Xtensa 3/10] Add initial version of the Xtensa backend..
Jul 16 2019, 4:04 PM · Restricted Project, Restricted Project
appcs added a reviewer for D64829: [Xtensa 3/10] Add initial version of the Xtensa backend.: appcs.
Jul 16 2019, 4:00 PM · Restricted Project, Restricted Project
appcs added inline comments to D64827: [Xtensa 2/10] Add Xtensa ELF definitions..
Jul 16 2019, 3:59 PM · Restricted Project, Restricted Project
appcs added a reviewer for D64827: [Xtensa 2/10] Add Xtensa ELF definitions.: appcs.
Jul 16 2019, 3:57 PM · Restricted Project, Restricted Project
appcs accepted D64826: [Xtensa 1/10] Recognize Xtensa in triple parsing code..
Jul 16 2019, 3:57 PM · Restricted Project, Restricted Project
appcs added a reviewer for D64826: [Xtensa 1/10] Recognize Xtensa in triple parsing code.: appcs.
Jul 16 2019, 3:51 PM · Restricted Project, Restricted Project

Dec 7 2018

appcs added a comment to D53185: [analyzer] Implement a prototype checker for detecting Year 2038 related issues..

Thank you @Szelethus for the guidance. Thank you @NoQ for further input. Please see inline:

Dec 7 2018, 3:18 PM

Nov 28 2018

appcs added a comment to D53185: [analyzer] Implement a prototype checker for detecting Year 2038 related issues..

ping

Nov 28 2018, 2:41 PM

Nov 1 2018

appcs added a comment to D53185: [analyzer] Implement a prototype checker for detecting Year 2038 related issues..

Let's talk about more general topics: Your checker's name is "TimeChecker", not "Y2K38Checker" or something similar, do you have plans on expanding this checker to cover more time.h related functionality?

Nov 1 2018, 4:23 PM
appcs added inline comments to D53185: [analyzer] Implement a prototype checker for detecting Year 2038 related issues..
Nov 1 2018, 10:50 AM
appcs added inline comments to D53185: [analyzer] Implement a prototype checker for detecting Year 2038 related issues..
Nov 1 2018, 10:37 AM

Oct 31 2018

appcs added inline comments to D53185: [analyzer] Implement a prototype checker for detecting Year 2038 related issues..
Oct 31 2018, 3:22 PM
appcs updated the diff for D53185: [analyzer] Implement a prototype checker for detecting Year 2038 related issues..

Incorporated code review feedback; thank you @Szelethus and @george.karpenkov

Oct 31 2018, 3:13 PM

Oct 22 2018

appcs added inline comments to D53185: [analyzer] Implement a prototype checker for detecting Year 2038 related issues..
Oct 22 2018, 12:16 PM

Oct 11 2018

appcs retitled D53185: [analyzer] Implement a prototype checker for detecting Year 2038 related issues. from Implement a prototype checker for detecting Year 2038 related issues. to Implement a prototype checker in the Clang Static Analyzer for detecting Year 2038 related issues..
Oct 11 2018, 9:30 PM
appcs updated subscribers of D53185: [analyzer] Implement a prototype checker for detecting Year 2038 related issues..
Oct 11 2018, 9:24 PM
appcs abandoned D53124: Register TimeChecker with Clang Static Analyzer.

Re-spun with feedback received into: D53185

Oct 11 2018, 9:22 PM
appcs added a reviewer for D53185: [analyzer] Implement a prototype checker for detecting Year 2038 related issues.: george.karpenkov.
Oct 11 2018, 9:17 PM
appcs updated the summary of D53185: [analyzer] Implement a prototype checker for detecting Year 2038 related issues..
Oct 11 2018, 9:14 PM
appcs updated the summary of D53185: [analyzer] Implement a prototype checker for detecting Year 2038 related issues..
Oct 11 2018, 9:14 PM
appcs updated the summary of D53185: [analyzer] Implement a prototype checker for detecting Year 2038 related issues..
Oct 11 2018, 9:11 PM
appcs updated the summary of D53185: [analyzer] Implement a prototype checker for detecting Year 2038 related issues..
Oct 11 2018, 9:10 PM
appcs added a reviewer for D53185: [analyzer] Implement a prototype checker for detecting Year 2038 related issues.: Szelethus.

Thank you very much for the feedback. I have added a little functionality to demonstrate the working of a prototype checker for a some sample Y2K38 issues. There are some TODO's in the test files indicative of some subsequent checking that we will add.

Oct 11 2018, 9:10 PM
appcs created D53185: [analyzer] Implement a prototype checker for detecting Year 2038 related issues..
Oct 11 2018, 9:03 PM

Oct 10 2018

appcs created D53124: Register TimeChecker with Clang Static Analyzer.
Oct 10 2018, 8:18 PM

Jan 19 2017

appcs added a comment to D28075: MergeFunctions: Preserve debug info in thunks, under option -mergefunc-preserve-debug-info.

Thank you, everybody, for the valuable inputs.

Jan 19 2017, 11:12 AM

Jan 13 2017

appcs added a comment to D28075: MergeFunctions: Preserve debug info in thunks, under option -mergefunc-preserve-debug-info.

There are several options I could think of:
Either surfacing it through a driver option, enabling it at -Og, or benchmarking it and arguing to enable it by default at some optimization settings.

Jan 13 2017, 7:09 PM
appcs updated the diff for D28075: MergeFunctions: Preserve debug info in thunks, under option -mergefunc-preserve-debug-info.
  • Added documentation to source code on -mergefunc-preserve-debug-info behaviour and noted the difference from the base -mergefunc
  • Elaborated & consolidated comments in regards MergeFunctionsPDI for MergeFunctions::writeThunk()
  • Made suggested stylistic changes.
Jan 13 2017, 7:06 PM

Jan 12 2017

appcs added a comment to D28075: MergeFunctions: Preserve debug info in thunks, under option -mergefunc-preserve-debug-info.

Added some more stylistic changes.

Jan 12 2017, 4:51 PM

Jan 11 2017

appcs added a comment to D28075: MergeFunctions: Preserve debug info in thunks, under option -mergefunc-preserve-debug-info.

Is this good?

Jan 11 2017, 7:56 AM

Jan 6 2017

appcs added a comment to D28075: MergeFunctions: Preserve debug info in thunks, under option -mergefunc-preserve-debug-info.

Submitting ticked off items.

Jan 6 2017, 8:59 PM
appcs updated the diff for D28075: MergeFunctions: Preserve debug info in thunks, under option -mergefunc-preserve-debug-info.

Under -mergefunc-preserve-debug-info

  • A thunk's call site is preserved to point to the thunk when both occur within the same translation unit.
Jan 6 2017, 8:50 PM
appcs added a comment to D28075: MergeFunctions: Preserve debug info in thunks, under option -mergefunc-preserve-debug-info.

What should -mergefunc-preserve-debug-info do about the thunk's call to the shared implementation?
Mark as tail call or not? The existing -mergefunc behaviour is to mark it as a tail call. We could leave it as is,
unless someone specifically asks for a change under -mergefunc-preserve-debug-info; would that be OK?

I think marking it as a tail call makes most sense.

Jan 6 2017, 1:59 PM
appcs added a comment to D28075: MergeFunctions: Preserve debug info in thunks, under option -mergefunc-preserve-debug-info.

Call sites of the merged function occurring from within the TU of the merged function's definition will point directly to the shared implementation and call sites of the merged function that are external to the TU of the merged function's definition call the thunk for the merged function (which tail call's the shared implementation passing forward the incoming arguments). This is existing behaviour under -mergefunc and remains unchanged under -mergefunc-preserve-debug-info (except for the tail call part).

Is this the right thing to do, though? I would think that for better debugability users would always prefer the version with the thunk even in the current TU. Then again, subsequent optimization passes would likely inline the thunk and thus potentially undo most of the effect. Have you experimented with this variant? Would the debug info from the thunk survive?

Thank you; I agree that this greatly enhances the debug experience. Automatically modifying the call site with a direct call to the shared implementation instead of (what is now) the thunk, when within the TU, is confusing when debugging. Stepping into the thunk from the call site and from within the thunk, into the shared implementation keeps the debug experience uniform for internal and external call sites of functions that become thunks. (If you ask me, this is similar in spirit to not marking the thunk's call to the shared implementation as a tail call to help debugging - the full back trace is the same in both cases and otherwise, there is a certain element of surprise in both cases. Basically, we are saying that under -mergefunc-preserve-debug-info we trade off optimization partly (at -O0) to aid debugability, modifying the transformation slightly, to make the execution flow explicit).

Yes. I think this is the right approach. When using -mergefunc-preserve-debug-info we should always call the thunk even in the same TU.

Jan 6 2017, 10:11 AM

Jan 5 2017

appcs added a comment to D28075: MergeFunctions: Preserve debug info in thunks, under option -mergefunc-preserve-debug-info.

Call sites of the merged function occurring from within the TU of the merged function's definition will point directly to the shared implementation and call sites of the merged function that are external to the TU of the merged function's definition call the thunk for the merged function (which tail call's the shared implementation passing forward the incoming arguments). This is existing behaviour under -mergefunc and remains unchanged under -mergefunc-preserve-debug-info (except for the tail call part).

Is this the right thing to do, though? I would think that for better debugability users would always prefer the version with the thunk even in the current TU. Then again, subsequent optimization passes would likely inline the thunk and thus potentially undo most of the effect. Have you experimented with this variant? Would the debug info from the thunk survive?

Jan 5 2017, 6:21 PM

Jan 4 2017

appcs added a comment to D28075: MergeFunctions: Preserve debug info in thunks, under option -mergefunc-preserve-debug-info.
Thank you for your review, Adrian.
Jan 4 2017, 4:52 PM

Jan 3 2017

appcs added inline comments to D28075: MergeFunctions: Preserve debug info in thunks, under option -mergefunc-preserve-debug-info.
Jan 3 2017, 3:37 PM

Dec 22 2016

appcs retitled D28075: MergeFunctions: Preserve debug info in thunks, under option -mergefunc-preserve-debug-info from to MergeFunctions: Preserve debug info in thunks, under option -mergefunc-preserve-debug-info.
Dec 22 2016, 7:41 PM

Dec 14 2016

appcs added a comment to D22051: MergeSimilarFunctions: a code size pass to merge functions with small differences.

MergeSimilarFunctions demonstrates great improvement on Cisco software, so this is of great value to us. We have some improvements upon the base work to improve the compile time.

Dec 14 2016, 11:17 AM