Page MenuHomePhabricator

Please use GitHub pull requests for new patches. Phabricator shutdown timeline

jamieschmeiser (Jamie Schmeiser)
User

Projects

User does not belong to any projects.

User Details

User Since
Aug 14 2020, 11:14 AM (163 w, 2 d)

Recent Activity

May 25 2023

jamieschmeiser accepted D151170: [StandardInstrumentations] Add option to dump IR to a file on crash.

LGTM

May 25 2023, 6:08 AM · Restricted Project, Restricted Project

May 23 2023

jamieschmeiser requested changes to D151170: [StandardInstrumentations] Add option to dump IR to a file on crash.
May 23 2023, 6:39 AM · Restricted Project, Restricted Project

Jan 31 2023

jamieschmeiser abandoned D142255: [WIP] Loop peeling opportunity for identity operators.

I agree that the potential gain is limited and probably not worth the potential increase in code size. This patch captures the changes/discussion if someone else is interested in the future. I am abandoning this revision.

Jan 31 2023, 9:04 AM · Restricted Project, Restricted Project

Jan 27 2023

jamieschmeiser added a comment to D142255: [WIP] Loop peeling opportunity for identity operators.

@nikic, thank you for your comments. I too have concerns about this proposal. I agree that the benefits are marginal but I would like to address some of your comments.

Jan 27 2023, 9:53 AM · Restricted Project, Restricted Project

Jan 24 2023

jamieschmeiser added inline comments to D142255: [WIP] Loop peeling opportunity for identity operators.
Jan 24 2023, 11:17 AM · Restricted Project, Restricted Project

Jan 20 2023

jamieschmeiser requested review of D142255: [WIP] Loop peeling opportunity for identity operators.
Jan 20 2023, 1:58 PM · Restricted Project, Restricted Project

Jan 11 2023

jamieschmeiser added a comment to D141156: Merge lines in print-changed by showing simple changes in place in the line.

This will show some of the differences:

Jan 11 2023, 7:51 AM · Restricted Project, Restricted Project

Jan 6 2023

jamieschmeiser requested review of D141156: Merge lines in print-changed by showing simple changes in place in the line.
Jan 6 2023, 1:18 PM · Restricted Project, Restricted Project

Dec 13 2022

jamieschmeiser accepted D139898: [StandardInstrumentations] Handle initial IR before checking if IR is interesting.

I was surprised that your change would require test changes--and that probably points out a bug in my initial design. Only the TextChangeReporter unwraps the module to always use the module in handleInitialIR. This should be lifted out to the call-site so it is consistant with all handlers. It doesn't make sense that the initial IR changes based on filtering (ie, which one prints first). But that is not germaine to this change so...

Dec 13 2022, 6:27 AM · Restricted Project, Restricted Project

Dec 12 2022

jamieschmeiser accepted D139720: [clang][PPC] Checking Unknown Values Passed to -mcpu.

LGTM

Dec 12 2022, 12:37 PM · Restricted Project, Restricted Project
jamieschmeiser added a comment to D139720: [clang][PPC] Checking Unknown Values Passed to -mcpu.

I'm fine with handling the return for common as a separate change, if necessary.
Is the error produced now because it passes back the incorrect option rather than quietly changing it to something appropriate as it did before?

Dec 12 2022, 12:13 PM · Restricted Project, Restricted Project
jamieschmeiser updated subscribers of D139720: [clang][PPC] Checking Unknown Values Passed to -mcpu.
Dec 12 2022, 7:10 AM · Restricted Project, Restricted Project
jamieschmeiser requested changes to D139720: [clang][PPC] Checking Unknown Values Passed to -mcpu.
Dec 12 2022, 5:50 AM · Restricted Project, Restricted Project

Dec 5 2022

jamieschmeiser committed rG455530425e07: Reland "A new hidden option exec-on-ir-change=exe that calls exe each time IR… (authored by jamieschmeiser).
Reland "A new hidden option exec-on-ir-change=exe that calls exe each time IR…
Dec 5 2022, 11:11 AM · Restricted Project, Restricted Project
jamieschmeiser closed D110776: A new hidden option exec-on-ir-change=exe that calls exe after each time IR changes.
Dec 5 2022, 11:11 AM · Restricted Project, Restricted Project
jamieschmeiser committed rG2b6683fd5f74: Expand loop peeling phi computation to handle binary ops and casts (authored by jamieschmeiser).
Expand loop peeling phi computation to handle binary ops and casts
Dec 5 2022, 9:15 AM · Restricted Project, Restricted Project
jamieschmeiser closed D138719: Expand loop peeling phi computation to handle binary ops and casts.
Dec 5 2022, 9:15 AM · Restricted Project, Restricted Project
jamieschmeiser updated the diff for D110776: A new hidden option exec-on-ir-change=exe that calls exe after each time IR changes.

ErrorOr isn't really appropriate here because the string was produced when the error occurred and it was None otherwise, which is the opposite of what happens with ErrorOr. It isn't desirable to create the array of filenames using ErrorOr since you want to reuse the existing ones or a large trace will open too many temporary files. However, your comment does point out that this return is inconsistent and awkward. I've changed these functions to return an std::error_code, which is more consistent. It loses the different error strings previously produced, but this code is only used for debugging anyway. Also, I changed the function for creating the temporary files to clean up any it succeeded in creating if an error occurs.

Dec 5 2022, 8:53 AM · Restricted Project, Restricted Project

Dec 2 2022

jamieschmeiser added a comment to D133614: handling invalidated passes in change printers.

ping

Dec 2 2022, 11:24 AM · Restricted Project, Restricted Project
jamieschmeiser abandoned D88515: Add dot-cfg-changes style view to the llvm-diff tool.

This is out of date and would require reworking.

Dec 2 2022, 11:23 AM · Restricted Project, Restricted Project
jamieschmeiser updated the diff for D110776: A new hidden option exec-on-ir-change=exe that calls exe after each time IR changes.

@aeubanks When I first reverted this because it failed on windows, I wasn't sure what to do to fix it and it fell off my radar.
I have updated it to reflect changes since then (the tests were updated to reflect changes to specifying filtering, functions have moved, etc). I've made a few minor changes, such as the functions for managing the temp files return Optional strings now and the tempfile name is different to reflect the multiple usage. I've also added a lit.local.cfg file to ensure that '/bin/cat' exists, which should fix the original windows failure that caused the revert. Can you please take a look to see that everything is still okay? Thanks.

Dec 2 2022, 11:08 AM · Restricted Project, Restricted Project
jamieschmeiser added inline comments to D138456: Add new option choices for print-changed that do not output llc changes..
Dec 2 2022, 8:14 AM · Restricted Project, Restricted Project
jamieschmeiser accepted D135658: demangle OptFunction trace names.

LGTM

Dec 2 2022, 7:20 AM · Restricted Project, Restricted Project, Restricted Project
jamieschmeiser updated the diff for D138719: Expand loop peeling phi computation to handle binary ops and casts.

Respond to review comments: remove unnecessary asserts

Dec 2 2022, 7:03 AM · Restricted Project, Restricted Project
jamieschmeiser added a comment to D135658: demangle OptFunction trace names.

Is the mode change on check-time-trace-sections.py intended? I did a count of python files under llvm-project and < 10% are 755. I'm not saying it is better one way or the other; I'm just ensuring it is intentional.

Dec 2 2022, 5:58 AM · Restricted Project, Restricted Project, Restricted Project

Nov 25 2022

jamieschmeiser requested review of D138719: Expand loop peeling phi computation to handle binary ops and casts.
Nov 25 2022, 7:34 AM · Restricted Project, Restricted Project
jamieschmeiser committed rGbe1ff1fe58b0: [NFC] Refactor loop peeling code for calculating phi invariance. (authored by jamieschmeiser).
[NFC] Refactor loop peeling code for calculating phi invariance.
Nov 25 2022, 6:07 AM · Restricted Project, Restricted Project
jamieschmeiser closed D138232: [NFC] Refactor loop peeling code for calculating phi invariance..
Nov 25 2022, 6:07 AM · Restricted Project, Restricted Project

Nov 23 2022

jamieschmeiser added inline comments to D138232: [NFC] Refactor loop peeling code for calculating phi invariance..
Nov 23 2022, 10:44 AM · Restricted Project, Restricted Project
jamieschmeiser updated the diff for D138232: [NFC] Refactor loop peeling code for calculating phi invariance..

@mkazantsev, I have addressed your concerns.
Regarding your comments concerning the use of infinity (as an unsigned) vs Optional<unsigned>: perhaps it's just me, but I found the use of Optional<unsigned> confusing and it became more confusing when I incorporated the max iterations into the algorithm. It isn't clear from the code at this point, but when one expands the algorithm to include binary operators, one wants to avoid computing the number of peels unnecessarily when the first operand has already reached (or surpassed) the maximum allowable peels. This is why I simplified the code to use an unsigned and placed it all within protected members of the class, wrapping it with the Optional<unsigned> on coming out, to both clarify the algorithm (especially when considering max iterations) and also to prevent people monkeying with the unsigned value (as you pointed out as a possibility in your comments). Your criticism of the constant name Infinity is valid--it is misleading. In response, I have hidden the Optional<unsigned> behind a type called PeelCount, used a constant Unknown (which is None) rather than Infinity and kept the math in protected functions. I think this makes the code easier to understand and also addresses your preference for using Optional<unsigned>.

Nov 23 2022, 10:31 AM · Restricted Project, Restricted Project

Nov 22 2022

jamieschmeiser updated the diff for D138232: [NFC] Refactor loop peeling code for calculating phi invariance..

I have split out the refactoring of the code so there are no intended
functional changes. I have added the test case as requested, showing
the current, limited functionality. After this lands, I will post a
patch with the expanded functionality.

Nov 22 2022, 2:27 PM · Restricted Project, Restricted Project
jamieschmeiser added inline comments to D138456: Add new option choices for print-changed that do not output llc changes..
Nov 22 2022, 5:31 AM · Restricted Project, Restricted Project

Nov 21 2022

jamieschmeiser requested review of D138456: Add new option choices for print-changed that do not output llc changes..
Nov 21 2022, 1:03 PM · Restricted Project, Restricted Project

Nov 17 2022

jamieschmeiser requested review of D138232: [NFC] Refactor loop peeling code for calculating phi invariance..
Nov 17 2022, 12:01 PM · Restricted Project, Restricted Project
jamieschmeiser added a comment to D133614: handling invalidated passes in change printers.

ping

Nov 17 2022, 11:57 AM · Restricted Project, Restricted Project

Nov 4 2022

jamieschmeiser abandoned D136285: Bad optimization with alloca and intrinsic function stackrestore.
Nov 4 2022, 7:21 AM · Restricted Project, Restricted Project

Oct 28 2022

jamieschmeiser added inline comments to D136285: Bad optimization with alloca and intrinsic function stackrestore.
Oct 28 2022, 2:42 PM · Restricted Project, Restricted Project

Oct 27 2022

jamieschmeiser added a comment to D136285: Bad optimization with alloca and intrinsic function stackrestore.

I have verified that adding inalloca and removing tail call on stackrestore in my sample IR will fix the problem.

Oct 27 2022, 1:23 PM · Restricted Project, Restricted Project

Oct 26 2022

jamieschmeiser committed rGb115ba005030: [NFC] Introduce range based singleton searches for loop queries. (authored by jamieschmeiser).
[NFC] Introduce range based singleton searches for loop queries.
Oct 26 2022, 10:51 AM · Restricted Project, Restricted Project
jamieschmeiser closed D136261: Introduce range based singleton searches for loop queries.
Oct 26 2022, 10:51 AM · Restricted Project, Restricted Project

Oct 25 2022

jamieschmeiser retitled D136261: Introduce range based singleton searches for loop queries from Templates for singleton loop queries to Introduce range based singleton searches for loop queries.
Oct 25 2022, 9:44 AM · Restricted Project, Restricted Project
jamieschmeiser updated the diff for D136261: Introduce range based singleton searches for loop queries.

Respond to review comments. Remove templates and introduce
generic range-based singleton search routines.

Oct 25 2022, 9:43 AM · Restricted Project, Restricted Project

Oct 21 2022

jamieschmeiser added a comment to D136284: SROA should freeze undefs for loads with no prior stores.

Yes, I agree it is incomplete (aside from being incorrect here :-) I've just been asking to ensure that my understanding of freeze is correct. Thanks.

Oct 21 2022, 9:42 AM · Restricted Project, Restricted Project, Restricted Project
jamieschmeiser added a comment to D136284: SROA should freeze undefs for loads with no prior stores.

At least in C++, working with uninitialized memory is pretty much always immediate undefined behavior, see https://eel.is/c++draft/basic.indet for the relevant wording. The only exception are "copy-like" operations on unsigned character types, which comparisons do not fall under.

I believe the C specification is less clear cut about this, but Clang and LLVM assume basically the same to also hold for C code.

What version of the C++ standard is this? Every version that I have seen has basics as section 3 and I cannot find this section, nor anything similar. Section 6 is Statements.

That discussion is orthogonal to this patch.
This patch is not desired because it's not needed per the current LLVM IR semantics. If you want to change something, you need to start by proposing a change to the LLVM IR semantics. You'll need to justify why it's needed, why it's correct, the perf impact, how to make it backwards compatible, why it's better than the proposals over the table right now.

Anyway, a patch like this solves no problem. LLVM allows loads to be duplicated. Your patch does nothing to prevent that and to ensure all loads see the same value. The issue is way more complicated than what this patch implies.

Oct 21 2022, 8:29 AM · Restricted Project, Restricted Project, Restricted Project
jamieschmeiser abandoned D136284: SROA should freeze undefs for loads with no prior stores.

I checked with a member of the C++ standards committee and he verified that comparing an uninitialized value against itself is, indeed, undefined behaviour, in the general case. I am abandoning this revision.

Oct 21 2022, 8:10 AM · Restricted Project, Restricted Project, Restricted Project
jamieschmeiser added a comment to D136284: SROA should freeze undefs for loads with no prior stores.

At least in C++, working with uninitialized memory is pretty much always immediate undefined behavior, see https://eel.is/c++draft/basic.indet for the relevant wording. The only exception are "copy-like" operations on unsigned character types, which comparisons do not fall under.

I believe the C specification is less clear cut about this, but Clang and LLVM assume basically the same to also hold for C code.

Oct 21 2022, 7:27 AM · Restricted Project, Restricted Project, Restricted Project
jamieschmeiser added a comment to D136284: SROA should freeze undefs for loads with no prior stores.

The current behavior here is intentional -- in fact, LLVM will move towards returning poison for loads from uninitialized memory in the future (though precisely how this will happen is still uncertain, see https://discourse.llvm.org/t/rfc-making-bit-field-codegen-poison-compatible/63250 for some recent discussion on the topic).

Oct 21 2022, 6:44 AM · Restricted Project, Restricted Project, Restricted Project
jamieschmeiser added a comment to D136284: SROA should freeze undefs for loads with no prior stores.

However, multiple loads of the same memory must be equal

This premise is incorrect w.r.t. with current semantics, which say that loads return undef for uninit memory.

As Nikita mentioned we are working towards changing this semantics. The work is ongoing, but we pivoted to make a couple of improvements in other areas to avoid perf regressions. For example, see the clang !noundef patches.

Oct 21 2022, 6:22 AM · Restricted Project, Restricted Project, Restricted Project

Oct 19 2022

jamieschmeiser added a comment to D136285: Bad optimization with alloca and intrinsic function stackrestore.

I found the following in clang/lib/CodeGen/CGCall.cpp:

// Insert a stack save if we're going to need any inalloca args.                                                                                                                                                                                            
if (hasInAllocaArgs(CGM, ExplicitCC, ArgTypes)) {
  assert(getTarget().getTriple().getArch() == llvm::Triple::x86 &&
         "inalloca only supported on x86");
  Args.allocateArgumentMemory(*this);
}
Oct 19 2022, 2:20 PM · Restricted Project, Restricted Project
jamieschmeiser added a comment to D136285: Bad optimization with alloca and intrinsic function stackrestore.

What does inalloca mean? It is only mentioned in the syntax line of https://llvm.org/docs/LangRef.html#alloca-instruction.

Oct 19 2022, 2:14 PM · Restricted Project, Restricted Project
jamieschmeiser added a comment to D136285: Bad optimization with alloca and intrinsic function stackrestore.

I think it will fail even if stackrestore is not tail call because it will still not pass the !isStaticAlloca query

Oct 19 2022, 1:28 PM · Restricted Project, Restricted Project
jamieschmeiser requested review of D136285: Bad optimization with alloca and intrinsic function stackrestore.
Oct 19 2022, 12:44 PM · Restricted Project, Restricted Project
jamieschmeiser requested review of D136284: SROA should freeze undefs for loads with no prior stores.
Oct 19 2022, 12:21 PM · Restricted Project, Restricted Project, Restricted Project
jamieschmeiser added a reviewer for D136261: Introduce range based singleton searches for loop queries: Restricted Project.
Oct 19 2022, 11:44 AM · Restricted Project, Restricted Project
jamieschmeiser requested review of D136261: Introduce range based singleton searches for loop queries.
Oct 19 2022, 8:14 AM · Restricted Project, Restricted Project
jamieschmeiser added a comment to D133614: handling invalidated passes in change printers.

ping

Oct 19 2022, 7:57 AM · Restricted Project, Restricted Project

Sep 19 2022

jamieschmeiser added a comment to D133662: [Clang] Change -ftime-trace storing path and support multiple compilation jobs.

Great. Also, something you may want to consider, either as part of or after you land this code. This really is a specific instance of a more generic problem: setting up option handling for something that saves results in a file for each compilation. This is equally useful for other things such as listings and could also be used by something like print-changed (which currently just outputs to the stream), opt stats reporting, etc. This code could be organized as a function (or possibly an object, depends...) that takes a string for the extension, a lambda/template for the virtual call on whether to add the option to a tool so that off-handling, platform-isms, and where files are saved would all be captured neatly and would be re-usable. InferTimeTrace and getPath, off-loading, platform-isms would be captured in a generic call that would look something like (in this instance)

PerFileTraceGenerator(".json",
     [](Tool &T, Args &Args)->bool{ return T->supportsTimeTrace() && Args.hasArg(options::OPT_ftime_trace, options::OPT_ftime_trace_EQ; },
     "-ftime-trace=");

Each option that needs per file output would just call this function appropriately.

Sep 19 2022, 6:48 AM · Restricted Project, Restricted Project

Sep 18 2022

jamieschmeiser added a comment to D133662: [Clang] Change -ftime-trace storing path and support multiple compilation jobs.

I had an email exchange with @dongjunduo about this situation. He was a GSoC student that @Whitney and I were mentoring for the past summer. He agrees that your approach is cleaner. There appears to be two parts to your work. First, you implemented the determining and passing of the options differently, and secondly, you improved the handling of off-loading and system specific file handling. Based on your earlier response, we proposed to him the following and he agrees that it seems appropriate. Could you please add comments to https://reviews.llvm.org/D131469 and he will work with you to change his code to reflect searching for -o and using the virtual functions. Then, if @MaskRay agrees, he can land his code and finish up his GSoC work. You can then add your extensions of off-loading and file-handling. Is this acceptable to you?

Sep 18 2022, 8:15 AM · Restricted Project, Restricted Project

Sep 13 2022

jamieschmeiser added a comment to D133662: [Clang] Change -ftime-trace storing path and support multiple compilation jobs.

I'm a little confused as to what is being proposed here. Is this building on D131469 or is it an alternative? It seems that there are portions of the code from D131469 included in these changes, which implies that you are building on it. I think this patch is premature in that the other patch has not yet landed (@MaskRay has asked for revisions that @dongjunduo has made but is waiting for review). When that patch has landed, this could be reposted based on those changes.

Sep 13 2022, 11:46 AM · Restricted Project, Restricted Project

Sep 9 2022

jamieschmeiser added a comment to D131960: [IR] Reimplement time tracing of NewPassManager by PassInstrumentation framework.

I agree. We could time the pass, figure out whether it changed anything (outside of the timing) and then decide how to save the timing info for the pass.

Sep 9 2022, 7:12 PM · Restricted Project, Restricted Project
jamieschmeiser added a comment to D131960: [IR] Reimplement time tracing of NewPassManager by PassInstrumentation framework.

@aeubanks An interesting idea. I'll give it some thought... it should be doable by combining this with the logic in the change printers but I'd be concerned that the detection of the changes would become a hotspot itself. Perhaps a double run, the first to determine which passes make changes and then a recompile to time those passes that are then known to make changes.

Sep 9 2022, 1:54 PM · Restricted Project, Restricted Project
jamieschmeiser requested review of D133614: handling invalidated passes in change printers.
Sep 9 2022, 1:45 PM · Restricted Project, Restricted Project
jamieschmeiser added a comment to D131960: [IR] Reimplement time tracing of NewPassManager by PassInstrumentation framework.

or do you mean noting which passes don't make changes and calculating the time spent in those passes? That would combine this work with the print-changed type filters...

Sep 9 2022, 1:23 PM · Restricted Project, Restricted Project
jamieschmeiser added a comment to D131960: [IR] Reimplement time tracing of NewPassManager by PassInstrumentation framework.

@aeubanks Not sure what you mean... something like print-changed=no-change? That would different from this work but we can continue the discussion.

Sep 9 2022, 1:18 PM · Restricted Project, Restricted Project
jamieschmeiser added a comment to D131960: [IR] Reimplement time tracing of NewPassManager by PassInstrumentation framework.

I have delivered https://reviews.llvm.org/D133587 so you should be good to go without the FIXME or changes.

Sep 9 2022, 10:56 AM · Restricted Project, Restricted Project
jamieschmeiser committed rG5e3ac7969039: Loop names used in reporting can grow very large (authored by jamieschmeiser).
Loop names used in reporting can grow very large
Sep 9 2022, 10:45 AM · Restricted Project, Restricted Project, Restricted Project
jamieschmeiser closed D133587: Loop names used in reporting can grow very large.
Sep 9 2022, 10:45 AM · Restricted Project, Restricted Project, Restricted Project
jamieschmeiser added a comment to D131960: [IR] Reimplement time tracing of NewPassManager by PassInstrumentation framework.

As discussed, change your code to use the name of the loop or call getIRName and add a FIXME comment to resolve the 2 pieces of code. https://reviews.llvm.org/D133587 has been created to fix the generation of the loop names and fix the lit tests.

Sep 9 2022, 10:06 AM · Restricted Project, Restricted Project
jamieschmeiser requested review of D133587: Loop names used in reporting can grow very large.
Sep 9 2022, 10:03 AM · Restricted Project, Restricted Project, Restricted Project

Sep 7 2022

jamieschmeiser accepted D131960: [IR] Reimplement time tracing of NewPassManager by PassInstrumentation framework.

A couple of nits left: small edits to comments and the change of "...() == nullptr" to "!...()" and it is good to go.
LGTM.

Sep 7 2022, 6:39 AM · Restricted Project, Restricted Project

Sep 6 2022

jamieschmeiser added inline comments to D131960: [IR] Reimplement time tracing of NewPassManager by PassInstrumentation framework.
Sep 6 2022, 11:58 AM · Restricted Project, Restricted Project
jamieschmeiser added inline comments to D131960: [IR] Reimplement time tracing of NewPassManager by PassInstrumentation framework.
Sep 6 2022, 5:27 AM · Restricted Project, Restricted Project
jamieschmeiser requested changes to D131960: [IR] Reimplement time tracing of NewPassManager by PassInstrumentation framework.
Sep 6 2022, 5:17 AM · Restricted Project, Restricted Project

Sep 2 2022

jamieschmeiser accepted D131469: [Clang] change default storing path of `-ftime-trace`.

If the source is specified with a partial path, I would expect the .json file to be in the same directory as the source, so dir1/f.C and dir2/f.C would result in dir1/f.json and dir2/f.json. But this can be a later improvement. Let's get this landed.

Sep 2 2022, 12:27 PM · Restricted Project, Restricted Project

Sep 1 2022

jamieschmeiser added inline comments to D133055: [MachineFunctionPass] Support -filter-passes for -print-changed.
Sep 1 2022, 5:57 AM · Restricted Project, Restricted Project

Aug 23 2022

jamieschmeiser added a comment to D131960: [IR] Reimplement time tracing of NewPassManager by PassInstrumentation framework.

You want the timing extensions to be the last ones called before the pass and the first ones at the end of the pass so that the overhead of the other callbacks is not included in the timings. Does PassInstrumentationCallbacks have the ability to specify that the pass be registered as the first callback? If not, add a bool parameter that allows it to be placed at the front with a default of false, and then put all timing extensions at the end, with the parameter being set to true for the registering of the atEnd callback (to put them as the first ones called). Also, add a comment that the timing ones should be last for this reason.

Aug 23 2022, 5:14 AM · Restricted Project, Restricted Project

Aug 22 2022

jamieschmeiser added inline comments to D131469: [Clang] change default storing path of `-ftime-trace`.
Aug 22 2022, 7:03 AM · Restricted Project, Restricted Project

Aug 18 2022

jamieschmeiser added a comment to D131960: [IR] Reimplement time tracing of NewPassManager by PassInstrumentation framework.

Considering that getIRName is only used for identifying the IR, do you even need the change?

Aug 18 2022, 9:08 AM · Restricted Project, Restricted Project

Aug 17 2022

jamieschmeiser added inline comments to D131960: [IR] Reimplement time tracing of NewPassManager by PassInstrumentation framework.
Aug 17 2022, 8:13 AM · Restricted Project, Restricted Project
jamieschmeiser added a comment to D131469: [Clang] change default storing path of `-ftime-trace`.

Good progress.

Aug 17 2022, 8:01 AM · Restricted Project, Restricted Project

Aug 11 2022

jamieschmeiser added a comment to D131469: [Clang] change default storing path of `-ftime-trace`.

You should not have debugging information in code that is up for review. If this is debugging information that you plan to leave in for future purposes (which I doubt is the case here), you need to protect it so that it isn't active unless some option is set. For example, see LLVM_DEBUG code that lives in various opt routines.

Aug 11 2022, 6:02 AM · Restricted Project, Restricted Project

Aug 8 2022

jamieschmeiser accepted D130596: [StandardInstrumentations] Handle case where block order changes.

LGTM. Thanks.

Aug 8 2022, 5:50 AM · Restricted Project, Restricted Project

Aug 5 2022

jamieschmeiser added a comment to D130596: [StandardInstrumentations] Handle case where block order changes.

Sorry for the delay, missed this before going on vacation.

Aug 5 2022, 12:02 PM · Restricted Project, Restricted Project

Jul 27 2022

jamieschmeiser accepted D130587: [StandardInstrumentations] Assign names to basic blocks without names.

I'm wondering what would happen (after this change) if a function were to re-order the blocks (without any other change). EG, the first 2 blocks in the function are swapped in the function list of blocks with no other change. I suspect that this would then indicate that the change was the original block 1 was replaced with the new block 2 and the original block 2 was replaced with the new block 1 with the corresponding changes to the branches. I think that may be unavoidable since the comparisons are string based. Besides, it is better than crashing...

Jul 27 2022, 1:40 PM · Restricted Project, Restricted Project

Jul 26 2022

jamieschmeiser accepted D130434: [MachineFunctionPass] Support -print-changed and -print-changed=quiet.

Since you have the before and after versions, I expect you could get -print-changed=diff and cdiff without too much difficulty by calling the diff routine that the opt print-changed options use, if you want to expand this functionality.

Jul 26 2022, 6:46 AM · Restricted Project, Restricted Project

Jul 11 2022

jamieschmeiser accepted D128048: Add a new clang option "-ftime-trace=<value>".

LGTM. Thanks.

Jul 11 2022, 8:20 AM · Restricted Project, Restricted Project

Jun 24 2022

jamieschmeiser added a comment to D128048: Add a new clang option "-ftime-trace=<value>".

You may add -ftime-trace=<file> instead of introducing a new spelling.

Jun 24 2022, 7:48 AM · Restricted Project, Restricted Project

Jun 23 2022

jamieschmeiser added inline comments to D128048: Add a new clang option "-ftime-trace=<value>".
Jun 23 2022, 12:36 PM · Restricted Project, Restricted Project

Jun 22 2022

jamieschmeiser added a comment to D128048: Add a new clang option "-ftime-trace=<value>".

Can you please use git rebase -i to collapse all the changes into a single change? If this isn't done, it is difficult to know what is being reviewed as the changes only show the differences since your last revision, not all of the changes.

I can see all the changes, make sure your link is not suffix with "new", or check the history tab to see what is it comparing.

Jun 22 2022, 8:58 AM · Restricted Project, Restricted Project
jamieschmeiser added a comment to D128048: Add a new clang option "-ftime-trace=<value>".

Can you please use git rebase -i to collapse all the changes into a single change? If this isn't done, it is difficult to know what is being reviewed as the changes only show the differences since your last revision, not all of the changes.

Jun 22 2022, 6:39 AM · Restricted Project, Restricted Project

Jun 17 2022

jamieschmeiser requested changes to D128048: Add a new clang option "-ftime-trace=<value>".

This is a good start. You should mention in the summary that when -c is not specified, the compiler is invoked with -o pointing at /tmp so the .json file will currently be placed there, which is confusing.

Jun 17 2022, 10:50 AM · Restricted Project, Restricted Project

Jun 6 2022

jamieschmeiser committed rG41778e3dc5f4: [NFC] Change lit test for print-changed=dot-cfg to use regular expression (authored by jamieschmeiser).
[NFC] Change lit test for print-changed=dot-cfg to use regular expression
Jun 6 2022, 12:54 PM · Restricted Project, Restricted Project
jamieschmeiser closed D126876: Change lit test for print-changed=dot-cfg to use regular expression.
Jun 6 2022, 12:53 PM · Restricted Project, Restricted Project

Jun 3 2022

jamieschmeiser committed rGefbf0136b410: Only issue warning for subtraction involving null pointers on live code paths (authored by jamieschmeiser).
Only issue warning for subtraction involving null pointers on live code paths
Jun 3 2022, 7:12 AM · Restricted Project, Restricted Project
jamieschmeiser closed D126816: Only issue warning for subtraction involving null pointers on live code paths.
Jun 3 2022, 7:12 AM · Restricted Project, Restricted Project

Jun 2 2022

jamieschmeiser updated the diff for D126816: Only issue warning for subtraction involving null pointers on live code paths.

Respond to review comments: add cpp test.

Jun 2 2022, 7:10 AM · Restricted Project, Restricted Project
jamieschmeiser requested review of D126876: Change lit test for print-changed=dot-cfg to use regular expression.
Jun 2 2022, 5:45 AM · Restricted Project, Restricted Project

Jun 1 2022

jamieschmeiser requested review of D126816: Only issue warning for subtraction involving null pointers on live code paths.
Jun 1 2022, 1:20 PM · Restricted Project, Restricted Project

Mar 8 2022

jamieschmeiser added a comment to D86657: Add new hidden option -print-on-crash that prints out IR that caused opt pipeline to crash.

Thanks!

Mar 8 2022, 10:35 AM · Restricted Project, Restricted Project

Dec 16 2021

jamieschmeiser reopened D110776: A new hidden option exec-on-ir-change=exe that calls exe after each time IR changes.

Failing build: https://lab.llvm.org/buildbot/#/builders/42/builds/2800

Dec 16 2021, 7:56 AM · Restricted Project, Restricted Project