Page MenuHomePhabricator

probinson (Paul Robinson)
User

Projects

User Details

User Since
May 9 2013, 11:10 AM (332 w, 3 d)

Recent Activity

Fri, Sep 20

probinson added a child revision for D67842: SSP: [1/3] revert r363169: D67844: SSP: [2/3] Refactor an if-dyn_cast chain to switch on opcode. NFC.
Fri, Sep 20, 10:47 AM · Restricted Project
probinson added a parent revision for D67844: SSP: [2/3] Refactor an if-dyn_cast chain to switch on opcode. NFC: D67842: SSP: [1/3] revert r363169.
Fri, Sep 20, 10:47 AM · Restricted Project
probinson added a child revision for D67844: SSP: [2/3] Refactor an if-dyn_cast chain to switch on opcode. NFC: D67845: SSP: [3/3] cmpxchg and addrspacecast can now trigger stack protection.
Fri, Sep 20, 10:47 AM · Restricted Project
probinson added a parent revision for D67845: SSP: [3/3] cmpxchg and addrspacecast can now trigger stack protection: D67844: SSP: [2/3] Refactor an if-dyn_cast chain to switch on opcode. NFC.
Fri, Sep 20, 10:47 AM · Restricted Project
probinson created D67845: SSP: [3/3] cmpxchg and addrspacecast can now trigger stack protection.
Fri, Sep 20, 10:44 AM · Restricted Project
probinson created D67844: SSP: [2/3] Refactor an if-dyn_cast chain to switch on opcode. NFC.
Fri, Sep 20, 10:41 AM · Restricted Project
probinson created D67842: SSP: [1/3] revert r363169.
Fri, Sep 20, 10:35 AM · Restricted Project

Thu, Sep 19

probinson added a comment to D67563: Debug Info: Add support for named constants.

I wonder, then, about a "constexpr T&" - maybe that's closer to a "true constant" - you can't take its address? (references have no address, as far as the language is concerned - maybe not-even-constexpr T& is a "true constant" (but only allows "address constants" not other things like integers to have similar handling)).

Thu, Sep 19, 9:38 AM · Restricted Project, debug-info
probinson added a comment to D67563: Debug Info: Add support for named constants.

What's a "true named constant" as opposed to, say, C++ (or C's) "const int X = 3;" (well, I guess const int X could be defined by a runtime expression, - OK, what about "constexpr int X = 3;" then?)

Thu, Sep 19, 5:54 AM · Restricted Project, debug-info

Mon, Sep 9

probinson added a comment to D67097: [DWARF] Check for format mismatch between CU and Range List Table..

FTR, I have also been thinking that perhaps the dumper should be more tolerant of this kind of thing, attempting to report more of what is present rather than throwing a hissy fit when things aren't perfect. But I'm just noting that for the record. What this patch does is consistent with how the rest of lib/DebugInfo/DWARF behaves, and revisiting that behavior is well beyond the scope of this patch.

Mon, Sep 9, 10:40 AM · Restricted Project, debug-info
probinson added a comment to D67097: [DWARF] Check for format mismatch between CU and Range List Table..

Ah right, the actual patch. I think it would be better to also report the base of the unit that referenced the range list, if that's not too inconvenient.
What you have here is fine with me, apart from that question.

Mon, Sep 9, 10:20 AM · Restricted Project, debug-info

Fri, Sep 6

probinson added a comment to D67276: [DWARF] Add a unit test for DWARFUnit::getLength()..

As written, this will work only for little-endian. There are other unittests that are bi-endian that you ought to be able to use for examples; if nothing else, bail out after checking sys::IsLittleEndianHost (but a bi-endian test would be better).

Fri, Sep 6, 9:13 AM · Restricted Project, debug-info
probinson added a comment to D67097: [DWARF] Check for format mismatch between CU and Range List Table..

Not sure I understand the snark at the C++ committee, but anyway.

Fri, Sep 6, 8:57 AM · Restricted Project, debug-info

Thu, Sep 5

probinson added a comment to D67097: [DWARF] Check for format mismatch between CU and Range List Table..

I'm not sure if using "OffsetIn" in the diagnostic is the right thing to do.

Honestly the fact that DWARFv5 specifies that the "rnglist base address" (& loclist bast address, and str_offsets base) point to the beginning of the list rather than the start of the header is super quirky and I don't think it's helpful to users to highlight that. (though I doubt DWARFvNext would bother fixing this - it'sjust quirky, not super problematic for any practical reasons I can think of)

IIRC this is one of Cary's, and argued that it could be directly used to index into section, instead of you having to step past the header to use it. Anyway we let him get away with it, for better or worse.

Meh, I can't imagine the (Start+Header)[N] would be too expensive (& Start+Header could be computed once/ahead of time easily enough).

Thu, Sep 5, 11:57 AM · Restricted Project, debug-info

Wed, Sep 4

probinson accepted D66643: [DWARF] Support DWARF64 in DWARFListTableHeader..

LGTM

Wed, Sep 4, 10:37 AM · Restricted Project, debug-info
probinson added a comment to D67097: [DWARF] Check for format mismatch between CU and Range List Table..

I'm not sure if using "OffsetIn" in the diagnostic is the right thing to do.

Honestly the fact that DWARFv5 specifies that the "rnglist base address" (& loclist bast address, and str_offsets base) point to the beginning of the list rather than the start of the header is super quirky and I don't think it's helpful to users to highlight that. (though I doubt DWARFvNext would bother fixing this - it'sjust quirky, not super problematic for any practical reasons I can think of)

Wed, Sep 4, 10:07 AM · Restricted Project, debug-info
probinson added a comment to D64672: [X86] Prevent passing vectors of __int128 as <X x i128> in llvm IR.

It looks to me like the patch does things the New Way only for Linux and NetBSD, so for PS4 backward compatibility I am okay with it.

Wed, Sep 4, 6:49 AM · Restricted Project

Fri, Aug 30

probinson added a comment to D67009: [TargetLowering][PS4] Add sincos(f) lib functions when target is PS4.

I'm happy if Simon's happy.

Fri, Aug 30, 10:13 AM · Restricted Project

Thu, Aug 29

probinson added a comment to D66141: [FileCheck] Forbid using var defined on same line.

I am okay with saying you cannot reference a variable in the same CHECK directive where it is defined.
There's the workaround of splitting such a directive into a CHECK and CHECK-SAME; it is not always identical (see my FileCheck Follies talk) but will usually work.

Thu, Aug 29, 6:48 AM · Restricted Project

Wed, Aug 28

probinson added a comment to D66746: [LiveDebugValues] Omit entry values for DBG_VALUEs with pre-existing expressions.

I have interpreted the size as meaning the byte size of the DWARF block that the operation will cover. Assuming that, at the time of running LiveDebugValues I don't think there is a good way to query the size of the block that the entry value will cover; we don't know that until we actually emit the DWARF, as far as I can tell. That is why I have assumed that a hard coded operand of 1 is emitted there, with the assumption that only simple register location descriptions are supported.

I think it would be reasonable to use the number of opcodes in the DIExpression in LLVM IR and only substitute the number of bytes in AsmPrinter.

Wed, Aug 28, 6:26 AM · Restricted Project, debug-info
probinson added inline comments to D66865: [DWARFVerifier] Verify GNU extensions of call site DWARF symbols.
Wed, Aug 28, 6:17 AM · Restricted Project, debug-info

Tue, Aug 27

probinson added a comment to D66328: [DebugInfo] Add debug location to stubs generated by CGDeclCXX and mark them as artificial.

I don't see a test for the __cxx_global_array_dtor case?

It is actually the arraydestroy.* loop that is covered (generated by CodeGenFunction::emitArrayDestroy), please see debug-info-destroy-helper.cpp below.

Tue, Aug 27, 8:22 AM · Restricted Project, debug-info, Restricted Project
probinson added a comment to D66328: [DebugInfo] Add debug location to stubs generated by CGDeclCXX and mark them as artificial.

I don't see a test for the __cxx_global_array_dtor case?

Tue, Aug 27, 7:32 AM · Restricted Project, debug-info, Restricted Project

Aug 23 2019

probinson added inline comments to D66643: [DWARF] Support DWARF64 in DWARFListTableHeader..
Aug 23 2019, 8:43 AM · Restricted Project, debug-info

Aug 22 2019

probinson added a comment to D63713: Add error handling to the DataExtractor class.

All the API overloads that take a Cursor should have doxygen descriptions.
No other complaints.

Aug 22 2019, 7:16 AM · Restricted Project
probinson updated subscribers of D66574: [lit] Make internal diff work in pipelines.

+Maggie who did the original diff implementation, AFAICT.

Aug 22 2019, 7:00 AM · Restricted Project

Aug 21 2019

probinson added a comment to D65697: [lit] Fix internal env calling env.
In D65697#1640109, @rnk wrote:

Most LLVM test suites are configured to use the internal shell on Windows. I think what's happening is that users happen to have an external copy of diff that gets called when it appears in a pipeline, or they are prefixed by not. The external diff is called, and things "work out" and we never noticed.

Are these patches going to break those tests on Windows? We now see diff anywhere in a pipeline, and we reject its appearance in a pipeline.

Aug 21 2019, 3:11 PM · Restricted Project
probinson added a comment to D65697: [lit] Fix internal env calling env.

Do people know why an internal diff was implemented originally? Is there a portability or performance issue?

Aug 21 2019, 8:29 AM · Restricted Project

Aug 20 2019

probinson added a comment to D65697: [lit] Fix internal env calling env.

I found one example of redirecting diff's output (to /dev/null).

Aug 20 2019, 1:03 PM · Restricted Project
probinson added a comment to D65697: [lit] Fix internal env calling env.

Searching llvm and clang tests for uses of diff, mostly they are diffing two generated files (e.g. diff %t1.ll %t2.ll).
There are diffs against canned files in an Inputs directory, and some of those tests pipe the generated output to diff's stdin. There are a number of these in clang/test/Analysis that diff output against canned XML results, so these are not small outputs you can readily verify with FileCheck.
I noticed some invocations passing -aub options to diff.
I found one example of redirecting diff's output (to /dev/null).

Aug 20 2019, 12:47 PM · Restricted Project
probinson added a comment to D66352: Debug Info: Support for DW_AT_export_symbols for anonymous structs.

Looking at the dwarfstd.org examples, without the flag (e.g., for DWARF v4) we should emit something like this:

<1> DW_TAG_struct_type
        DW_AT_name "A"
  <2> DW_TAG_struct_type
          // no DW_AT_name
      <3> DW_TAG_member
            DW_AT_name "x"
            DW_AT_type (int)
  <4> DW_TAG_struct_type
          // no DW_AT_name
      <5> DW_TAG_member
            DW_AT_name "y"
            DW_AT_type (int)
  <6> DW_TAG_member // this is A's member for the anonymous struct
          // no DW_AT_name
          DW_AT_type <2>
  <7> DW_TAG_member // this is A's member for the unnamed struct
          DW_AT_name "C"
          DW_AT_type <4>

which means the way the consumer knows to promote the members of <2> to the scope of <1> is that its only reference (from <6>) is via a member that is unnamed, and it knows NOT to promote the members of <4> because the reference to it is from a member that IS named.

Aug 20 2019, 9:54 AM · Restricted Project, debug-info
probinson added inline comments to D66328: [DebugInfo] Add debug location to stubs generated by CGDeclCXX and mark them as artificial.
Aug 20 2019, 8:24 AM · Restricted Project, debug-info, Restricted Project
probinson accepted D66472: [DWARF] Adjust return type of DWARFUnit::getLength()..

Is DWARFCompileUnit::dump() really the only consumer of this API? I would have thought maybe somewhere in dsymutil would be using it.

Aug 20 2019, 7:14 AM · Restricted Project
probinson added a comment to D66465: [DWARF] Fix reading 64-bit DWARF type units..

Hi @MaskRay please remember to post a comment along with approval so Phab will send an email to the commits list. Thanks!

Aug 20 2019, 7:00 AM · Restricted Project
probinson added a comment to D66421: [DWARF] Fix DWARFUnit::getDebugInfoSize() for 64-bit DWARF..

Hi @aprantl please remember to post a comment along with approval, so that Phab will send an email to the commits list. Thanks!

Aug 20 2019, 6:57 AM · Restricted Project

Aug 19 2019

probinson accepted D66293: [lit] Check for accidental external command calls.

I was actually misunderstanding how lit/tests/lit.cfg fit in, and reacting to the comment about piping in lit.cfg. The top-level description actually looks okay. It makes sense to verify lit itself is behaving as desired in lit's own test suite, and not add these external commands to the PATH of all tests.
LGTM

Aug 19 2019, 3:34 PM · Restricted Project
probinson added a comment to D65697: [lit] Fix internal env calling env.

This all seems plausible but I would rather defer approval to someone who actually understood Python.

Aug 19 2019, 2:43 PM · Restricted Project
probinson added a comment to D66293: [lit] Check for accidental external command calls.

IIUC this means if anyone tries to write a RUN line that pipes something to diff or cd or whatever, it would explicitly fail. That's probably a good move.
I suspect that : is not a legal filename on Windows. I'm not able to create such a file from the Windows CMD shell, and grepping the llvm and clang test suites for 'RUN:.* :' doesn't turn up anything that tries to use : as a command. Maybe we should just eliminate that command?

Aug 19 2019, 2:36 PM · Restricted Project
probinson added a comment to D66328: [DebugInfo] Add debug location to stubs generated by CGDeclCXX and mark them as artificial.

"debug-info-no-location.cpp" is an extremely generic name for a very specific case. "debug-info-atexit-stub.cpp" would be better.

Aug 19 2019, 11:19 AM · Restricted Project, debug-info, Restricted Project

Aug 8 2019

probinson added a comment to D65121: [FileCheck] Make FILECHECK_OPTS useful for its test suite.

Another possibility: %ProtectFileCheckOutput

Aug 8 2019, 12:19 PM · Restricted Project
probinson added a comment to D65697: [lit] Fix internal env calling env.

Those files under fake-externals... are those symlinks? IIUC that doesn't work so well on Windows. I admit I haven't tried downloading your patch to verify.

Aug 8 2019, 12:10 PM · Restricted Project
probinson added a comment to D63713: Add error handling to the DataExtractor class.

This probably needs to be rebased now that D64006 is in?

Aug 8 2019, 11:51 AM · Restricted Project
probinson accepted D65121: [FileCheck] Make FILECHECK_OPTS useful for its test suite.
Aug 8 2019, 11:50 AM · Restricted Project
probinson added a comment to D65121: [FileCheck] Make FILECHECK_OPTS useful for its test suite.

More bikeshedding: The new substitution could be called %TestFileCheckOutput (thus not abbreviating Output to Out) which helps it to be more self-documenting.
Or, it could be called %TestFileCheckEnv because its function is to set the environment, and it can optionally take an environment-variable definition as an argument. (Sorry if I already suggested this and we decided against.)
Everything else about this looks fine, LGTM once the substitution's name is settled.

Aug 8 2019, 11:50 AM · Restricted Project
probinson added a comment to D65914: [Dwarf] DW_TAG_unspecified_type is a type tag..

FYI. You can derive the complete list by scanning for DW_AT_type in DWARF 5 Appendix A, Table A.1.

? not really. Any tag that is a type qualifier will have DW_AT_type, but also anything else that simply has a type (DW_TAG_constant, etc).

I do think that the regex DW_TAG_*_type will find all of them.

I'm probably misunderstanding something here, but DW_TAG_constant *is* listed in Appendix A as allowing a DW_AT_type attribute, so are all the DW_TAG_*_type tags.

Aug 8 2019, 11:12 AM · Restricted Project
probinson added a comment to D65914: [Dwarf] DW_TAG_unspecified_type is a type tag..

FYI. You can derive the complete list by scanning for DW_AT_type in DWARF 5 Appendix A, Table A.1.

Aug 8 2019, 10:33 AM · Restricted Project
probinson added a comment to D65914: [Dwarf] DW_TAG_unspecified_type is a type tag..

This doesn't seem complete. And it's hard to tell, because the list is mostly but not entirely in numeric order.
Mostly it's missing the new v5 type tags (coarray, dynamic, atomic, immutable) which is probably my fault, but some older ones also appear to be missing (restrict, shared).

Aug 8 2019, 8:28 AM · Restricted Project

Aug 6 2019

probinson added a comment to D65410: [PassManager] First Pass implementation at -O1 pass pipeline.

The goal from the original email was:

Aug 6 2019, 10:58 AM · Restricted Project, Restricted Project

Aug 2 2019

probinson committed rG89683e9dd7db: [doc] Give a workaround for a FileCheck regex that ends in a brace. (authored by probinson).
[doc] Give a workaround for a FileCheck regex that ends in a brace.
Aug 2 2019, 9:12 AM
probinson committed rL367689: [doc] Give a workaround for a FileCheck regex that ends in a brace..
[doc] Give a workaround for a FileCheck regex that ends in a brace.
Aug 2 2019, 9:07 AM

Aug 1 2019

probinson added a comment to D65121: [FileCheck] Make FILECHECK_OPTS useful for its test suite.

Getting back to this after a long month on first-responder-to-bugs duty. I know you're trying to get the lit/env issue sorted out, but I had a half hour available to look at this.

Aug 1 2019, 11:01 AM · Restricted Project
probinson added a comment to D65156: [lit] Protect full test suite from FILECHECK_OPTS.

Is it feasible to turn off the FILECHECK variables for the entire 'lit' suite including its sample sub-suites?
I think that's an acceptable limitation. It is always possible to go back to the old way of debugging tests by running commands individually.

Aug 1 2019, 8:17 AM · Restricted Project
probinson added a comment to D65156: [lit] Protect full test suite from FILECHECK_OPTS.

Thanks for tracking down the internal/external thing.
not as an internal command might work; note that it takes an optional --crash which does something to make crash detection more reliable.

Aug 1 2019, 8:05 AM · Restricted Project

Jul 31 2019

probinson added a comment to D65156: [lit] Protect full test suite from FILECHECK_OPTS.

Thanks. Is the outermost lit call here running lit's internal shell or an external shell? (The lit call I mean is the one running lit's test suite and complaining about that command line, which is a lit call within lit's test suite.)

Jul 31 2019, 5:55 AM · Restricted Project

Jul 30 2019

probinson added a comment to D65436: [docs] Add note about git version to git-llvm section.

See the llvm-commits replies to r365917; at least one code owner is stuck with 1.8.3.

Jul 30 2019, 7:04 AM · Restricted Project, Restricted Project

Jul 29 2019

probinson added inline comments to D64209: [DWARF] Make DWARFDataExtractor possible to be used with both 32- and 64-bit offsets..
Jul 29 2019, 1:34 PM · Restricted Project

Jul 24 2019

probinson accepted D65156: [lit] Protect full test suite from FILECHECK_OPTS.

LGTM

Jul 24 2019, 8:21 AM · Restricted Project
probinson added a reviewer for D58531: [clang] Specify type of pthread_create builtin: jdoerfert.

We've started running into this too in building the PS4 system. +jdoerfert who added pthread_create to the builtin list.

Jul 24 2019, 7:37 AM · Restricted Project
probinson committed rGc7046c91cd10: Silence a conversion warning after r366887. NFC (authored by probinson).
Silence a conversion warning after r366887. NFC
Jul 24 2019, 7:16 AM
probinson committed rL366906: Silence a conversion warning after r366887. NFC.
Silence a conversion warning after r366887. NFC
Jul 24 2019, 7:15 AM

Jul 23 2019

probinson accepted D60388: FileCheck [8/12]: Define numeric var from expr.
Jul 23 2019, 12:54 PM · Restricted Project
probinson added a comment to D60388: FileCheck [8/12]: Define numeric var from expr.

A couple of minor things, otherwise LGTM.

Jul 23 2019, 9:10 AM · Restricted Project
probinson added a comment to D65121: [FileCheck] Make FILECHECK_OPTS useful for its test suite.

The lit part seems like a good thing. It's unfortunate that lit/test/lit.cfg needs to know about FileCheck environment variables, but them's the breaks.

If this issue were present in more test suites, I'd be tempted to extend FileCheck with a command-line option to encapsulate this use case in some manner. As things are, that doesn't seem worth the trouble to me, but maybe others feel differently.

Jul 23 2019, 8:35 AM · Restricted Project
probinson added a comment to D65121: [FileCheck] Make FILECHECK_OPTS useful for its test suite.

Regarding the FileCheck part, it should be split off to its own patch and the mass modification of FileCheck's test suite incorporated, because it's hard to grasp the effects without seeing those.

Jul 23 2019, 8:17 AM · Restricted Project
probinson added a comment to D65121: [FileCheck] Make FILECHECK_OPTS useful for its test suite.

The lit part seems like a good thing. It's unfortunate that lit/test/lit.cfg needs to know about FileCheck environment variables, but them's the breaks. That bit should be its own patch, regardless of how we decide to handle FileCheck itself.

Jul 23 2019, 7:44 AM · Restricted Project

Jul 22 2019

probinson committed rGd2c0eefd5cb0: [X86] Remove const from some intrinsics that shouldn't have them (authored by probinson).
[X86] Remove const from some intrinsics that shouldn't have them
Jul 22 2019, 9:15 AM
probinson committed rL366699: [X86] Remove const from some intrinsics that shouldn't have them.
[X86] Remove const from some intrinsics that shouldn't have them
Jul 22 2019, 9:13 AM
probinson accepted D65039: [DWARF] Add named constants for reserved values of an initial length field..

LGTM

Jul 22 2019, 7:36 AM · Restricted Project

Jul 19 2019

probinson added a comment to D64971: [SafeStack] Insert the deref after the offset.
In D64971#1593796, @vsk wrote:

AFAICT the deref is always needed here: my understanding is that dbg.value has to describe the actual value of a variable, not its address (https://www.llvm.org/docs/LangRef.html#diexpression). I admit that keeping the deref may break the dwarf builder, but istm that indicates a bug in the dwarf builder.

Jul 19 2019, 10:23 AM · Restricted Project

Jul 18 2019

probinson added a comment to D64893: Ask confirmation when `git llvm push` will push multiple commits.

+1 for defaulting to No.
As a follow-up patch, maybe the script could offer to squash the commits for you.

Jul 18 2019, 10:59 AM · Restricted Project
probinson added a comment to D64923: [FileCheck]] Canonicalize caret location testing.

Readability is to some extent in the eye of the reader, so I've restrained myself to one suggestion. Otherwise this review is up to James.

Jul 18 2019, 9:34 AM · Restricted Project
probinson added inline comments to D60388: FileCheck [8/12]: Define numeric var from expr.
Jul 18 2019, 6:44 AM · Restricted Project
probinson added a comment to D63713: Add error handling to the DataExtractor class.

I have been reminded that there's also a desire to make DataExtractor work with 64-bit section sizes. Maybe Cursor should use a 64-bit offset (i.e., size_t not uint32_t), and then migrating from non-Cursor to Cursor APIs will also do the 64-bit transition? We need to bite that bullet at some point. (I kind of expect y'all to say, no way do that later, which is fine; mainly I wanted to refresh that in our collective minds.)

Jul 18 2019, 6:30 AM · Restricted Project
probinson added a comment to D64875: Add LLVM's LICENSE.txt file to the monorepo root.

@probinson I don't understand what you mean -- what's the issue you see with adding this file right now?

Jul 18 2019, 6:19 AM · Restricted Project

Jul 17 2019

probinson added a comment to D64875: Add LLVM's LICENSE.txt file to the monorepo root.

This is imposing a requirement on how downstream projects manage their license files. I'm not saying we can't (eventually) do it this way, but I'm quite sure that's not how Sony handles it right now.

Jul 17 2019, 11:50 AM · Restricted Project
probinson added a comment to D64544: [DWARF] Add more error handling to debug line parser..

How would you be able to emit a wrong form value for the MD5 hash from assembly? The directive (which I think you came up with?) is just md5 [data]? Deciding which form to emit is hard-coded in MCDwarf.

Jul 17 2019, 11:39 AM · Restricted Project
probinson added a comment to D64544: [DWARF] Add more error handling to debug line parser..

I was rather hoping for test coverage for all the new error messages this change introduced - is that unrealistic/impractical?

Yeah, the line table is especially tricky to hand-craft compared to checking in an object file. I think it technically can still be hand-crafted assembly (no line directives, etc, just a debug_line section with raw byte (etc) directives) - might be plausible & make it clearer what the input is? (checked in assembly, assembled with llvm-mc then run through llvm-dwarfdump to test the parsing)

Jul 17 2019, 10:58 AM · Restricted Project
probinson added a comment to D57400: Add a .gitignore file to the root that ignores any files outside of the project directories..

What about downstream users that have added directories in their local forks? Having git suddenly ignore them would be surprising. We are in that situation.

Jul 17 2019, 10:45 AM
probinson accepted D63713: Add error handling to the DataExtractor class.

I'm happy, but other people obviously have better eyesight than I do. Give Jonas and Blaikie a day to chime in, I think.

Jul 17 2019, 8:04 AM · Restricted Project

Jul 16 2019

probinson added inline comments to D60388: FileCheck [8/12]: Define numeric var from expr.
Jul 16 2019, 9:58 AM · Restricted Project
probinson added a comment to D63713: Add error handling to the DataExtractor class.

Minor stuff.

Jul 16 2019, 8:53 AM · Restricted Project

Jul 15 2019

probinson added reviewers for D64780: Disallow most calling convention attributes on PS4.: rnk, rjmccall.

This has my blessing as PS4 code owner, but I'd like other eyes on it with respect to how we've gone about it.
+ rnk, rjmccall as the most likely suspects.

Jul 15 2019, 5:31 PM · Restricted Project, Restricted Project
probinson accepted D64622: [DWARF] Fix the reserved values for unit length in DWARFDebugLine..

If you found the unittests cases because they failed, that's good enough for me.
LGTM

Jul 15 2019, 8:15 AM · Restricted Project
probinson added a comment to D64622: [DWARF] Fix the reserved values for unit length in DWARFDebugLine..

I mean, the purposes for the patches are different. This one fixes an existing flaw while your suggestion improves code quality. It is usually better not to intermix such different aims in one patch, no?

Jul 15 2019, 7:13 AM · Restricted Project

Jul 12 2019

probinson added a comment to D64622: [DWARF] Fix the reserved values for unit length in DWARFDebugLine..

Right, but maybe that worths another patch?

Jul 12 2019, 11:19 AM · Restricted Project
probinson added a comment to D64630: [DebugInfo] Address performance regression with r364515.

With ASAN, packs of up to 800 DBG_VALUEs in a row appear (for that file)

Jul 12 2019, 11:10 AM · Restricted Project
probinson added a comment to D64622: [DWARF] Fix the reserved values for unit length in DWARFDebugLine..

This magic number appears in many places; it should have a symbolic name in llvm/include/llvm/BinaryFormat/Dwarf.h, in the LLVMConstants enumeration.

Jul 12 2019, 11:03 AM · Restricted Project

Jul 11 2019

probinson committed rG2cb5c46e670c: [clangd] Fix MSVC build failure. (authored by probinson).
[clangd] Fix MSVC build failure.
Jul 11 2019, 4:49 PM
probinson committed rL365844: [clangd] Fix MSVC build failure..
[clangd] Fix MSVC build failure.
Jul 11 2019, 4:47 PM
probinson added a comment to D64033: Add column info for inline sites.

LGTM2

Jul 11 2019, 3:12 PM · Restricted Project

Jul 10 2019

probinson added a comment to D64033: Add column info for inline sites.

If the change from DW_AT_MIPS_linkage_name to DW_AT_linkage_name made you think the older .o files were MIPS, more likely it is a change in the default DWARF version; older versions of DWARF did not have DW_AT_linkage_name.

Jul 10 2019, 10:46 AM · Restricted Project
probinson added a comment to D64231: [FileCheck] Simplify numeric variable interface.

I was under the impression that an assertion in the public interface of a library API was not a great idea.

Oops my bad, I forgot about that comment. I think only what's in the FileCheck class should be considered public API. The rest ought to be moved to a different header file and is private. FileCheckNumericVariable is an implementation details IMO.

Jul 10 2019, 10:34 AM · Restricted Project

Jul 9 2019

probinson added a comment to D64033: Add column info for inline sites.

I guess we can leave the .s file tests alone. Most of them look like they were derived from real -S output and it would be pedantically consistent to update them, but they don't have to be.
@JDevlieghere once you sort out the odr-member-functions the patch is fine with me. I am mildly curious why all three .o files got smaller.

Jul 9 2019, 5:19 PM · Restricted Project
probinson committed rG9e04b532dfe2: [CXX] Exercise all paths through these tests. (authored by probinson).
[CXX] Exercise all paths through these tests.
Jul 9 2019, 1:50 PM
probinson committed rL365555: [CXX] Exercise all paths through these tests..
[CXX] Exercise all paths through these tests.
Jul 9 2019, 1:49 PM
probinson closed D63894: [CXX] Exercise all paths through these tests.
Jul 9 2019, 1:48 PM · Restricted Project, Restricted Project
probinson added inline comments to D63894: [CXX] Exercise all paths through these tests.
Jul 9 2019, 12:58 PM · Restricted Project, Restricted Project
probinson added inline comments to D64189: Allow llc to run passes under the new pass manager one at a time..
Jul 9 2019, 9:39 AM · Restricted Project
probinson added a comment to D63894: [CXX] Exercise all paths through these tests.

there is no reason to think those paths are tested elsewhere.

Jul 9 2019, 9:39 AM · Restricted Project, Restricted Project
probinson added a comment to D63894: [CXX] Exercise all paths through these tests.

I would've assumed these conditionals were added by Sony folks for their change in default dialect - that doesn't necessarily mean these tests are needed upstream (the functionality may be tested elsewhere)

Jul 9 2019, 9:32 AM · Restricted Project, Restricted Project