search for: lgtm

> > Only a single LGTM is required. Reviewers are expected to only LGTM > patches they're confident in their knowledge of. Reviewers may review > and provide suggestions, but explicitly defer LGTM to someone else. > This is encouraged and a good way for new contributors to learn the code. Whilst I get wha...

On Mon, Nov 18, 2019 at 4:53 PM Finkel, Hal J. via llvm-dev <llvm-dev at lists.llvm.org> wrote: > On 11/18/19 4:29 AM, James Henderson wrote: >> >> Only a single LGTM is required. Reviewers are expected to only LGTM >> patches they're confident in their knowledge of. Reviewers may review >> and provide suggestions, but explicitly defer LGTM to someone else. >> This is encouraged and a good way for new contributors to learn the code. >...

I've read "LLVM Code-Review Policies and Practices," but I remain unsure of a couple of things. Do I always wait for an actual "LGTM", or can people approve the patch for submission in other ways? What happens when a patch is approved but then there are additional review comments? Should the patch be submitted as is, then a follow-up patch submitted, or should the additional review comments be incorporated first?

...know you worked hard to make sure that updating a revision doesn't send email unless there is text typed into one of the boxes Manuel, but I think we should by default put some text into a box (and send the email unless the user deletes that text) when accepting a revision. Otherwise, the final LGTM can accidentally happen on Phab and not reach the mailing list (D4178 for example). Thoughts? -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20140627/02e483e2/attachment.html>

Hi Rafael, comment explanation added now. Thanks. On Tue, Jan 14, 2014 at 8:31 PM, Rafael Espíndola < rafael.espindola at gmail.com> wrote: > LGTM with a comment explaining the issue. > > On 20 December 2013 03:50, Andrew MacPherson <andrew.macp at gmail.com> > wrote: > > Thanks Joerg. > > > > I've made the change you suggested and verified that it still works. I > think > > the noinline is still...

...cussion ever get a conclusion? I support enabling > pipefail. Fallout for out of tree users should be easy to fix. As we > learned from LLVM tests, almost all tests that start to fail actually > indicate a real problem that was hidden. So far I got some positive feedback, but no strong LGTM from someone in the area :-( > Dmitri > Cheers, Rafael

> On 2015 May 20, at 17:51, Duncan P. N. Exon Smith <dexonsmith at apple.com> wrote: > > >> On 2015 May 20, at 17:39, Duncan P. N. Exon Smith <dexonsmith at apple.com> wrote: >> >> With just those four patches, memory usage went *up* slightly. Add in >> the 5th patch (which does #2 below), and we get an overall memory drop >> of 4%. > >

...; >> > >> Hi Renato, > >> > >> Can you please approve the following revisions for backporting to 3.5.1: > >> > >> r216989: Only emit movw on ARMv6T2+ > >> r216990: Missing test from r216989 > >> > > > > These both LGTM, go ahead and merge into the 3.5 branch. > > > > -Tom > > > >> -Dimitry > >> > > > > > > _______________________________________________ > > LLVM Developers mailing list > > LLVMdev at cs.uiuc.edu <mailto:LLVMdev at cs.uiuc.e...

At 8/16/2020 10:39 AM, Stefan Stipanovic wrote: >Hi Paul, > >I've read "LLVM Code-Review Policies and Practices," but I remain unsure of a couple of things. Do I always wait for an actual "LGTM", or can people approve the patch for submission in other ways? > >The patch is accepted through Phabricator with or without a message (the message is usually LGTM or something similar) Aha! I didn't see the green Accepted indicator in the upper-left. I should look around more caref...

...in FastISel pass (without failing back to DAG lowering). The new patch is attached, and the test case is not changed. Sorry for your inconvenience. Please have a look. Thanks for your help. Sincerely, Logan On Wed, Aug 21, 2013 at 10:52 PM, JF Bastien <jfb at google.com> wrote: > lgtm > > > On Wed, Aug 21, 2013 at 3:18 AM, Anton Korobeynikov < > anton at korobeynikov.info> wrote: > >> LGTM >> >> On Wed, Aug 21, 2013 at 1:51 PM, Logan Chien <tzuhsiang.chien at gmail.com> >> wrote: >> > Hi, >> > >> > I...

...atomic_helper_swap_state(struct drm_atomic_state *state, > > __for_each_private_obj(state, obj, obj_state, i, funcs) > funcs->swap_state(obj, &state->private_objs[i].obj_state); > + > + return 0; > } > EXPORT_SYMBOL(drm_atomic_helper_swap_state); Above hunks lgtm. > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index 7d416428bdc1..e211d703fe2e 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -13183,7 +13183,15 @@ static int intel_atomic_c...

I'm not very familiar with relocations but your fix looks the same as ARMTargetLowering::LowerCall, so from that perspective it lgtm (but I may be missing something). On Wed, Aug 21, 2013 at 8:04 AM, Logan Chien <tzuhsiang.chien at gmail.com>wrote: > Hi Anton and JF, > > Thanks for your review. After reading the source code more carefully, I > have come up with a different way fix this issue. We can simp...

...ccaaaabb" in > > if PCRE.matches m then ( > > let whole = PCRE.sub m 0 in > > Not really, my suggestion was to have a separate object representing > the result of a regex match -- much like other language have in their > regex APIs. > > OTOH, this solution LGTM as well: the result of the regex is not saved > in a thread-local variable, but directly in the same regex object, so > can be kept/used around, and it is GC'ed when not needed anymore. > If you could apply that change, that'd be a LGTM. But the problem still is this makes the if/...

...du] On Behalf Of JF Bastien Sent: Wednesday, August 21, 2013 12:53 PM To: Logan Chien Cc: llvmdev at cs.uiuc.edu Subject: Re: [LLVMdev] Broken PLT on ARM from R183966 I'm not very familiar with relocations but your fix looks the same as ARMTargetLowering::LowerCall, so from that perspective it lgtm (but I may be missing something). On Wed, Aug 21, 2013 at 8:04 AM, Logan Chien <tzuhsiang.chien at gmail.com<mailto:tzuhsiang.chien at gmail.com>> wrote: Hi Anton and JF, Thanks for your review. After reading the source code more carefully, I have come up with a different way fix th...

On Mon, Apr 20, 2015 at 1:44 AM, Shankar Easwaran <shankarke at gmail.com> wrote: > Attached patch fixes the issue. Thanks for the quick fix. The patch LGTM if you add a test case. Unfortunately yaml2obj does not support duplicated section names but we can use a binary input file. -- Simon Atanasyan

...; mentions Phabricator. I'd like to move Phabricator to be mentioned on > > this line before the other methods. > > > > Please let me know what you think. > > > > Thanks again, > > > > Hal > > A couple of additional things: > > Only a single LGTM is required. Reviewers are expected to only LGTM > patches they're confident in their knowledge of. Reviewers may review > and provide suggestions, but explicitly defer LGTM to someone else. > This is encouraged and a good way for new contributors to learn the code. > > There i...

On Feb 12, 2013, at 1:41 PM, Pekka Jääskeläinen <pekka.jaaskelainen at tut.fi> wrote: > Hi, > > Here it is, just synched against the latest LLVM trunk. Shall I commit this > now? LGTM. Please commit. > After committing, it could be worth some planning what is the best way to > provide an easy to use mechanism to "refresh" the parallel loop mem > access metadata (llvm.mem.parallel_loop_access) after optimizations that do not > render the loop to a serial...

...n the input I >> received here). Please let me know if this work looks mergeable. >> >> The documentation is only partially filled in, I'll add more details >> once support for Go is also merged (the current co-routine work I'm >> doing). >> > > LGTM. > > Evan, anything else? Nope. Evan > >> Thanks! >> > > Thanks! > Rafael

On Tue, Aug 6, 2013 at 5:55 PM, Peter Collingbourne <peter at pcc.me.uk> wrote: > Hi, > > If there are no further comments on the design below I intend to commit > my DFSan patches in a week. > I think it would be good to get Kostya's explicit sign-off on this before committing it, as he has been directing and overseeing the sanitizer work as a whole over the past year.

...> What purpose does the notion of "primitive" types serve anymore? Why don't we just abolish that from the lexicon and from the code? > > Hi Chris, > > The attached patch removes it from Type.h and updates the last users. > Is that what you were looking for? Yep, LGTM. It would also be nice to remove the mention from LangRef. -Chris