lebedev.ri (Roman Lebedev)
User

Projects

User does not belong to any projects.

User Details

User Since
Oct 27 2012, 6:35 AM (315 w, 3 d)

Recent Activity

Yesterday

lebedev.ri added inline comments to D54473: [sanitizers] Initial implementation for -fsanitize=init-locals.
Tue, Nov 13, 7:21 AM
lebedev.ri added reviewers for D54470: Avoid g++ warning spam: rnk, aaron.ballman, bogner.

+1, it's super noisy.

Tue, Nov 13, 5:13 AM

Mon, Nov 12

lebedev.ri added a comment to D54445: [llvm-exegesis] BitVectorVector: provide assign() method, use it..

I will cut this patchseries here for now.
Old walltime was around ~25sec, new is ~5.6sec, so -77.58%.

Mon, Nov 12, 11:51 PM
lebedev.ri added a comment to D54418: [llvm-exegesis] InstructionBenchmarkClustering::dbScan(): replace SetVector with custom BitVectorVector.

Thanks for your benchmark file!

llvm-exegesis -mode=analysis -analysis-epsilon=100000 -benchmarks-file=/tmp/Downloads/benchmarks.yaml -analysis-inconsistencies-output-file=/tmp/clusters.html > /tmp/new

https://reviews.llvm.org/D54442 alone (without your other optimization) reduces the time from 16s to 4.2s on my machine. I'll leave the const auto Neighbors = rangeQuery(Q); inlining and other stuff to you :)

Mon, Nov 12, 11:23 PM
lebedev.ri added a comment to D53949: [clang][CodeGen] Implicit Conversion Sanitizer: discover the world of CompoundAssign operators.

Ping.

Mon, Nov 12, 1:59 PM · Restricted Project, Restricted Project
lebedev.ri added a dependency for D54445: [llvm-exegesis] BitVectorVector: provide assign() method, use it.: D54418: [llvm-exegesis] InstructionBenchmarkClustering::dbScan(): replace SetVector with custom BitVectorVector.
Mon, Nov 12, 1:32 PM
lebedev.ri added a dependent revision for D54418: [llvm-exegesis] InstructionBenchmarkClustering::dbScan(): replace SetVector with custom BitVectorVector: D54445: [llvm-exegesis] BitVectorVector: provide assign() method, use it..
Mon, Nov 12, 1:32 PM
lebedev.ri created D54445: [llvm-exegesis] BitVectorVector: provide assign() method, use it..
Mon, Nov 12, 1:31 PM
lebedev.ri added a comment to D54418: [llvm-exegesis] InstructionBenchmarkClustering::dbScan(): replace SetVector with custom BitVectorVector.

I'm also not following...

I'm sorry that i'm not following what you do not follow :)

DBSCAN is a BFS/Dijkstra-like algorithm. a BitVector + reserved std::vector suffice.

Which is exactly what this BitVectorVector implements, as far as i can tell.
I do not understand what the question is. I'm sorry. Can you reformulate the question, please?

Maybe I should quote "talk is cheap" here... My idea is https://reviews.llvm.org/D54442 (I hope it won't interfere your other improvement)

I still do not understand what the question is.
Yes, that pretty much the same as this code.
Differences:

  • uses pop back, instead of pop front. my measurements so far always showed pop back to be worse.
  • Both data structures are inline, instead of abstracting them under BitVectorVector.
  • One very interesting point - initial insertion is a special case, no need to check in the set. I do expect that to be faster, let's see..

Can you suggest some way to get the large benchmark file as you have in /tmp/benchmarks.yaml? I need to experiment with it but it is still unclear to me why pop_back is slower.

Mon, Nov 12, 1:14 PM
lebedev.ri added a comment to D54418: [llvm-exegesis] InstructionBenchmarkClustering::dbScan(): replace SetVector with custom BitVectorVector.

I'm also not following...

I'm sorry that i'm not following what you do not follow :)

DBSCAN is a BFS/Dijkstra-like algorithm. a BitVector + reserved std::vector suffice.

Which is exactly what this BitVectorVector implements, as far as i can tell.
I do not understand what the question is. I'm sorry. Can you reformulate the question, please?

Maybe I should quote "talk is cheap" here... My idea is https://reviews.llvm.org/D54442 (I hope it won't interfere your other improvement)

Mon, Nov 12, 1:08 PM
lebedev.ri added a comment to D54442: [llvm-exegesis] Optimize ToProcess in dbScan.

That is more or less how it ends up looking in D54418, just without abstraction.

Mon, Nov 12, 1:02 PM
lebedev.ri added a comment to D54418: [llvm-exegesis] InstructionBenchmarkClustering::dbScan(): replace SetVector with custom BitVectorVector.

I'm also not following...

I'm sorry that i'm not following what you do not follow :)

Mon, Nov 12, 12:34 PM
lebedev.ri updated the diff for D54383: [llvm-exegesis] Analysis: writeMeasurementValue(): don't alloc string for double each time..
  • Bloated the code with temporary variables to explain everything.
  • Provide actual benchmark.
Mon, Nov 12, 3:54 AM
lebedev.ri added a comment to D54418: [llvm-exegesis] InstructionBenchmarkClustering::dbScan(): replace SetVector with custom BitVectorVector.

I'm not convinced you need a new type here - why don't you just use BitVector (or SparseBitVector?) and create a helper function for the insert/erase?

Mon, Nov 12, 3:17 AM
lebedev.ri added a comment to D54383: [llvm-exegesis] Analysis: writeMeasurementValue(): don't alloc string for double each time..

I'm not entirely convinced here:

  • The code is harder to read,

I can totally bloat it will some comments/variables.

Mon, Nov 12, 3:10 AM
lebedev.ri added a dependent revision for D54415: [llvm-exegesis] InstructionBenchmarkClustering::rangeQuery(): reserve for the upper bound of Neighbors: D54418: [llvm-exegesis] InstructionBenchmarkClustering::dbScan(): replace SetVector with custom BitVectorVector.
Mon, Nov 12, 3:02 AM
lebedev.ri added a dependency for D54418: [llvm-exegesis] InstructionBenchmarkClustering::dbScan(): replace SetVector with custom BitVectorVector: D54415: [llvm-exegesis] InstructionBenchmarkClustering::rangeQuery(): reserve for the upper bound of Neighbors.
Mon, Nov 12, 3:02 AM
lebedev.ri created D54418: [llvm-exegesis] InstructionBenchmarkClustering::dbScan(): replace SetVector with custom BitVectorVector.
Mon, Nov 12, 3:02 AM
lebedev.ri added a comment to D54381: [llvm-exegesis] InstructionBenchmarkClustering::dbScan(): use llvm::SetVector<> instead of ILLEGAL std::unordered_set<>.

When used to cluster points (measurements) this way with DBSCAN, the order how points are processed does not matter.

Are you implying that it is incorrect to use anything but non-unsorted set here?
I can't just use DenseSet here, it ends up begin at least one magnitude slower.
And we most certainly can't keep std::unordered_set<>.

Mon, Nov 12, 1:45 AM
lebedev.ri added inline comments to D54415: [llvm-exegesis] InstructionBenchmarkClustering::rangeQuery(): reserve for the upper bound of Neighbors.
Mon, Nov 12, 1:30 AM
lebedev.ri added inline comments to D54415: [llvm-exegesis] InstructionBenchmarkClustering::rangeQuery(): reserve for the upper bound of Neighbors.
Mon, Nov 12, 12:32 AM
lebedev.ri retitled D54393: [llvm-exegesis] Move InstructionBenchmarkClustering::isNeighbour() into header from [llvm-exegesis] Move InstructionBenchmarkClustering::() into header to [llvm-exegesis] Move InstructionBenchmarkClustering::isNeighbour() into header.
Mon, Nov 12, 12:29 AM
lebedev.ri added inline comments to D54393: [llvm-exegesis] Move InstructionBenchmarkClustering::isNeighbour() into header.
Mon, Nov 12, 12:27 AM
lebedev.ri added inline comments to D54388: [llvm-exegesis] InstructionBenchmarkClustering::rangeQuery(): use llvm::SmallVector<size_t, 0> for storage..
Mon, Nov 12, 12:09 AM
lebedev.ri added inline comments to D54390: [llvm-exegesis] InstructionBenchmarkClustering::rangeQuery(): write into llvm::SmallVectorImpl& output parameter.
Mon, Nov 12, 12:09 AM
lebedev.ri added a dependent revision for D54393: [llvm-exegesis] Move InstructionBenchmarkClustering::isNeighbour() into header: D54415: [llvm-exegesis] InstructionBenchmarkClustering::rangeQuery(): reserve for the upper bound of Neighbors.
Mon, Nov 12, 12:08 AM
lebedev.ri added a dependent revision for D54390: [llvm-exegesis] InstructionBenchmarkClustering::rangeQuery(): write into llvm::SmallVectorImpl& output parameter: D54415: [llvm-exegesis] InstructionBenchmarkClustering::rangeQuery(): reserve for the upper bound of Neighbors.
Mon, Nov 12, 12:08 AM
lebedev.ri added dependencies for D54415: [llvm-exegesis] InstructionBenchmarkClustering::rangeQuery(): reserve for the upper bound of Neighbors: D54393: [llvm-exegesis] Move InstructionBenchmarkClustering::isNeighbour() into header, D54388: [llvm-exegesis] InstructionBenchmarkClustering::rangeQuery(): use llvm::SmallVector<size_t, 0> for storage., D54390: [llvm-exegesis] InstructionBenchmarkClustering::rangeQuery(): write into llvm::SmallVectorImpl& output parameter.
Mon, Nov 12, 12:08 AM
lebedev.ri added a dependent revision for D54388: [llvm-exegesis] InstructionBenchmarkClustering::rangeQuery(): use llvm::SmallVector<size_t, 0> for storage.: D54415: [llvm-exegesis] InstructionBenchmarkClustering::rangeQuery(): reserve for the upper bound of Neighbors.
Mon, Nov 12, 12:08 AM
lebedev.ri created D54415: [llvm-exegesis] InstructionBenchmarkClustering::rangeQuery(): reserve for the upper bound of Neighbors.
Mon, Nov 12, 12:07 AM

Sun, Nov 11

lebedev.ri added a comment to D54390: [llvm-exegesis] InstructionBenchmarkClustering::rangeQuery(): write into llvm::SmallVectorImpl& output parameter.

LG. The issue is that there shouldn't be frequent SmallVector construction/destruction in the loop body.

Here comes my own question: when the specific vector type does not matter too much (as in this case), when shall we choose SmallVector and when std::vector?

Sun, Nov 11, 11:59 PM
lebedev.ri added inline comments to D54388: [llvm-exegesis] InstructionBenchmarkClustering::rangeQuery(): use llvm::SmallVector<size_t, 0> for storage..
Sun, Nov 11, 11:16 PM
lebedev.ri added a comment to D54382: [llvm-exegesis] Analysis::writeSnippet(): be smarter about memory allocations..

// FIXME: magic number.

The owner should give you a reasonable capacity :)

Sun, Nov 11, 2:08 PM
lebedev.ri updated the summary of D54393: [llvm-exegesis] Move InstructionBenchmarkClustering::isNeighbour() into header.
Sun, Nov 11, 10:17 AM
lebedev.ri added a dependency for D54393: [llvm-exegesis] Move InstructionBenchmarkClustering::isNeighbour() into header: D54390: [llvm-exegesis] InstructionBenchmarkClustering::rangeQuery(): write into llvm::SmallVectorImpl& output parameter.
Sun, Nov 11, 10:13 AM
lebedev.ri created D54393: [llvm-exegesis] Move InstructionBenchmarkClustering::isNeighbour() into header.
Sun, Nov 11, 10:13 AM
lebedev.ri added a dependent revision for D54390: [llvm-exegesis] InstructionBenchmarkClustering::rangeQuery(): write into llvm::SmallVectorImpl& output parameter: D54393: [llvm-exegesis] Move InstructionBenchmarkClustering::isNeighbour() into header.
Sun, Nov 11, 10:13 AM
lebedev.ri added a comment to D54381: [llvm-exegesis] InstructionBenchmarkClustering::dbScan(): use llvm::SetVector<> instead of ILLEGAL std::unordered_set<>.

I could be missing something, but I don't understand why ToProcess needs to be a set-like container since we're erasing elements as we go (ie the erased elements won't be duplicate checked on next insertion). We skip any that have been previously processed in the inner loop too, which seems like it's doing the same work the set would be doing.

Here i only improve the choice of the data structure, and don't do any algorithm-changing decisions.

Sun, Nov 11, 6:51 AM
lebedev.ri added a dependent revision for D54389: [llvm-exegesis] InstructionBenchmarkClustering::dbScan(): replace std::vector<> with std::deque<> in llvm::SetVector<>: D54390: [llvm-exegesis] InstructionBenchmarkClustering::rangeQuery(): write into llvm::SmallVectorImpl& output parameter.
Sun, Nov 11, 3:07 AM
lebedev.ri added a dependency for D54390: [llvm-exegesis] InstructionBenchmarkClustering::rangeQuery(): write into llvm::SmallVectorImpl& output parameter: D54389: [llvm-exegesis] InstructionBenchmarkClustering::dbScan(): replace std::vector<> with std::deque<> in llvm::SetVector<>.
Sun, Nov 11, 3:07 AM
lebedev.ri created D54390: [llvm-exegesis] InstructionBenchmarkClustering::rangeQuery(): write into llvm::SmallVectorImpl& output parameter.
Sun, Nov 11, 3:07 AM
lebedev.ri added a dependency for D54389: [llvm-exegesis] InstructionBenchmarkClustering::dbScan(): replace std::vector<> with std::deque<> in llvm::SetVector<>: D54388: [llvm-exegesis] InstructionBenchmarkClustering::rangeQuery(): use llvm::SmallVector<size_t, 0> for storage..
Sun, Nov 11, 1:16 AM
lebedev.ri added a dependent revision for D54388: [llvm-exegesis] InstructionBenchmarkClustering::rangeQuery(): use llvm::SmallVector<size_t, 0> for storage.: D54389: [llvm-exegesis] InstructionBenchmarkClustering::dbScan(): replace std::vector<> with std::deque<> in llvm::SetVector<>.
Sun, Nov 11, 1:16 AM
lebedev.ri created D54389: [llvm-exegesis] InstructionBenchmarkClustering::dbScan(): replace std::vector<> with std::deque<> in llvm::SetVector<>.
Sun, Nov 11, 1:16 AM
lebedev.ri added a dependency for D54388: [llvm-exegesis] InstructionBenchmarkClustering::rangeQuery(): use llvm::SmallVector<size_t, 0> for storage.: D54383: [llvm-exegesis] Analysis: writeMeasurementValue(): don't alloc string for double each time..
Sun, Nov 11, 1:02 AM
lebedev.ri added a dependent revision for D54383: [llvm-exegesis] Analysis: writeMeasurementValue(): don't alloc string for double each time.: D54388: [llvm-exegesis] InstructionBenchmarkClustering::rangeQuery(): use llvm::SmallVector<size_t, 0> for storage..
Sun, Nov 11, 1:02 AM
lebedev.ri created D54388: [llvm-exegesis] InstructionBenchmarkClustering::rangeQuery(): use llvm::SmallVector<size_t, 0> for storage..
Sun, Nov 11, 1:02 AM
lebedev.ri added a dependent revision for D54382: [llvm-exegesis] Analysis::writeSnippet(): be smarter about memory allocations.: D54383: [llvm-exegesis] Analysis: writeMeasurementValue(): don't alloc string for double each time..
Sun, Nov 11, 12:57 AM
lebedev.ri added a dependency for D54383: [llvm-exegesis] Analysis: writeMeasurementValue(): don't alloc string for double each time.: D54382: [llvm-exegesis] Analysis::writeSnippet(): be smarter about memory allocations..
Sun, Nov 11, 12:57 AM
lebedev.ri added a dependency for D54382: [llvm-exegesis] Analysis::writeSnippet(): be smarter about memory allocations.: D54381: [llvm-exegesis] InstructionBenchmarkClustering::dbScan(): use llvm::SetVector<> instead of ILLEGAL std::unordered_set<>.
Sun, Nov 11, 12:57 AM
lebedev.ri added a dependent revision for D54381: [llvm-exegesis] InstructionBenchmarkClustering::dbScan(): use llvm::SetVector<> instead of ILLEGAL std::unordered_set<>: D54382: [llvm-exegesis] Analysis::writeSnippet(): be smarter about memory allocations..
Sun, Nov 11, 12:57 AM

Sat, Nov 10

lebedev.ri created D54383: [llvm-exegesis] Analysis: writeMeasurementValue(): don't alloc string for double each time..
Sat, Nov 10, 1:57 PM
lebedev.ri created D54382: [llvm-exegesis] Analysis::writeSnippet(): be smarter about memory allocations..
Sat, Nov 10, 1:11 PM
lebedev.ri created D54381: [llvm-exegesis] InstructionBenchmarkClustering::dbScan(): use llvm::SetVector<> instead of ILLEGAL std::unordered_set<>.
Sat, Nov 10, 12:10 PM
lebedev.ri added a comment to D53771: [clang-tidy] Avoid C arrays check.

*Maybe* we should be actually diagnosing the each actual declaration, not just the typeloc.
Else if you use one typedef, and // NOLINT it, you will silence it for all the decls that use it.

Sat, Nov 10, 7:43 AM · Restricted Project
lebedev.ri added inline comments to D53771: [clang-tidy] Avoid C arrays check.
Sat, Nov 10, 7:34 AM · Restricted Project
lebedev.ri added inline comments to D53771: [clang-tidy] Avoid C arrays check.
Sat, Nov 10, 7:14 AM · Restricted Project
lebedev.ri updated the diff for D53771: [clang-tidy] Avoid C arrays check.

Use isVariableArrayType()

Sat, Nov 10, 7:14 AM · Restricted Project
lebedev.ri added a comment to D53771: [clang-tidy] Avoid C arrays check.

Comments? :)

Sat, Nov 10, 4:33 AM · Restricted Project
lebedev.ri added inline comments to D54344: [Clang][CodeGen][CXX]: Workaround __attribute((no_destroy)) crash/incorrect code generation..
Sat, Nov 10, 3:22 AM

Fri, Nov 9

lebedev.ri added a comment to D54349: [clang-tidy] new check 'readability-redundant-preprocessor'.

No one will know for sure what "pp" in "readability-redundant-pp" means.
I'd highly recommend to fully spell it out.

Fri, Nov 9, 1:28 PM · Restricted Project
lebedev.ri added a comment to D46662: [X86] condition branches folding for three-way conditional codes.
In D46662#1246781, @xur wrote:

Hi Andrea,

Thanks for running this test, and the explanation. Can you run the tests
on Bulldozer/Ryzen? I don't have access to these platforms. If I need to do
this in subtarget way, it would be good to know the performance there.

CC'ing @lebedev.ri and @GGanesh.
They should be able to help you with running those tests on Bulldozer/Ryzen. Unfortunately, I don't have access to those machines.

I *think* this should be fine on bdver2, as per https://www.agner.org/optimize/microarchitecture.pdf:

19.15 Branches and loops
The branch prediction mechanism is described on page 34. There is no longer any
restriction on the number of branches per 16 bytes of code that can be predicted efficiently.
The misprediction penalty is quite high because of a long pipeline.
Fri, Nov 9, 8:49 AM

Thu, Nov 8

lebedev.ri added a comment to D54262: [clang-tidy] Add `delete this` bugprone check (PR38741).

It's best to use clang-tidy/add_new_check.py tool.
You also need tests, and a note in releasenotes.

Thu, Nov 8, 7:29 AM · Restricted Project

Wed, Nov 7

lebedev.ri added a comment to D52695: [clang][Parse] Diagnose useless null statements (PR39111).

ping.
I wonder if @aaron.ballman has any opinion on this.

Wed, Nov 7, 12:17 AM

Tue, Nov 6

lebedev.ri added a comment to D54115: [InstCombine] do not shrink switch conditions to illegal types (PR29009).

Looks good.

Tue, Nov 6, 10:29 AM
lebedev.ri added a comment to D52421: [Sema] Diagnose parameter names that shadow inherited field names.

...

...

Ok, thank you for reassuring me that it is working as intended.

Tue, Nov 6, 10:03 AM
lebedev.ri added a comment to D52421: [Sema] Diagnose parameter names that shadow inherited field names.

And now i'm having concerns.

struct Base {
    void* f;
};
Tue, Nov 6, 9:36 AM
lebedev.ri accepted D54150: Adapt UBSan integer truncation tests to NetBSD.

Thanks!
That is the relaxation of the tests i was going to do.
It's kinda ugly, but well...

Tue, Nov 6, 6:15 AM · Restricted Project
lebedev.ri added inline comments to D53974: [clang-tidy] new check: bugprone-too-small-loop-variable.
Tue, Nov 6, 1:20 AM · Restricted Project

Mon, Nov 5

lebedev.ri added a comment to D50050: [AST] CastExpr: BasePathSize is not large enough..

@lebedev.ri @erichkeane The test fails for me on macOS whenever asan and ubsan are both enabled.
The failure is stack overflow at stack frame 943

Mon, Nov 5, 12:55 PM
lebedev.ri added a comment to D54115: [InstCombine] do not shrink switch conditions to illegal types (PR29009).

Please do reference the backend bug№ in the comments.

Mon, Nov 5, 11:08 AM
lebedev.ri added inline comments to D54115: [InstCombine] do not shrink switch conditions to illegal types (PR29009).
Mon, Nov 5, 10:44 AM
lebedev.ri added inline comments to D54115: [InstCombine] do not shrink switch conditions to illegal types (PR29009).
Mon, Nov 5, 10:30 AM
lebedev.ri created D54095: [X86] X86DAGToDAGISel::matchBitExtract(): extract 'lshr' from `X`.
Mon, Nov 5, 2:28 AM
lebedev.ri added inline comments to D53488: [clang-tidy] Improving narrowing conversions.
Mon, Nov 5, 1:12 AM · Restricted Project

Sun, Nov 4

lebedev.ri added inline comments to D53771: [clang-tidy] Avoid C arrays check.
Sun, Nov 4, 6:29 AM · Restricted Project
lebedev.ri updated the diff for D53771: [clang-tidy] Avoid C arrays check.

Better diag for VLA arrays.

Sun, Nov 4, 6:29 AM · Restricted Project
lebedev.ri added a comment to D53771: [clang-tidy] Avoid C arrays check.

(only comments, patch to follow)

Sun, Nov 4, 5:05 AM · Restricted Project
lebedev.ri added inline comments to D33672: [analyzer] INT50-CPP. Do not cast to an out-of-range enumeration checker.
Sun, Nov 4, 2:23 AM
lebedev.ri added inline comments to D33672: [analyzer] INT50-CPP. Do not cast to an out-of-range enumeration checker.
Sun, Nov 4, 2:12 AM
lebedev.ri added inline comments to D33672: [analyzer] INT50-CPP. Do not cast to an out-of-range enumeration checker.
Sun, Nov 4, 2:02 AM

Sat, Nov 3

lebedev.ri added a dependency for D53949: [clang][CodeGen] Implicit Conversion Sanitizer: discover the world of CompoundAssign operators: D50250: [clang][ubsan] Implicit Conversion Sanitizer - integer sign change - clang part.
Sat, Nov 3, 12:57 AM · Restricted Project, Restricted Project
lebedev.ri added a dependent revision for D50250: [clang][ubsan] Implicit Conversion Sanitizer - integer sign change - clang part: D53949: [clang][CodeGen] Implicit Conversion Sanitizer: discover the world of CompoundAssign operators.
Sat, Nov 3, 12:57 AM · Restricted Project, Restricted Project

Fri, Nov 2

lebedev.ri updated the diff for D53949: [clang][CodeGen] Implicit Conversion Sanitizer: discover the world of CompoundAssign operators.

And finish the test coverage.

Fri, Nov 2, 3:41 AM · Restricted Project, Restricted Project

Thu, Nov 1

lebedev.ri added a comment to D53949: [clang][CodeGen] Implicit Conversion Sanitizer: discover the world of CompoundAssign operators.

Regarding ++ and --, I think for now it's fine to just document that these aren't instrumented.

Thu, Nov 1, 11:56 PM · Restricted Project, Restricted Project
lebedev.ri updated the diff for D53949: [clang][CodeGen] Implicit Conversion Sanitizer: discover the world of CompoundAssign operators.

Added test coverage.
Please review :)

Thu, Nov 1, 6:38 AM · Restricted Project, Restricted Project
lebedev.ri added inline comments to D53936: [clang-tidy] More clearly separate public, check-facing APIs from internal ones..
Thu, Nov 1, 2:21 AM · Restricted Project

Wed, Oct 31

lebedev.ri added a comment to D53949: [clang][CodeGen] Implicit Conversion Sanitizer: discover the world of CompoundAssign operators.

I do not agree that ++ is performed on the original type. The C99 standard (6.5.3.1.2) appears to be very clear on this point: "The expression ++E is equivalent to (E+=1)."

Wed, Oct 31, 11:11 PM · Restricted Project, Restricted Project
lebedev.ri added a comment to D53949: [clang][CodeGen] Implicit Conversion Sanitizer: discover the world of CompoundAssign operators.

I do not agree that ++ is performed on the original type.

I was only talking about the IR.

Wed, Oct 31, 2:22 PM · Restricted Project, Restricted Project
lebedev.ri added a comment to D53949: [clang][CodeGen] Implicit Conversion Sanitizer: discover the world of CompoundAssign operators.

This patch doesn't appear to yet fix the "x++" or "x--" cases.

It won't, the increment/decrement happens on the original type, there is no cast there. https://godbolt.org/z/WuWA62
Those cases are for normal signed/unsigned overflow checks.

Wed, Oct 31, 2:11 PM · Restricted Project, Restricted Project
lebedev.ri added a comment to D53949: [clang][CodeGen] Implicit Conversion Sanitizer: discover the world of CompoundAssign operators.

This patch doesn't appear to yet fix the "x++" or "x--" cases.

Wed, Oct 31, 2:05 PM · Restricted Project, Restricted Project
lebedev.ri added a comment to D53949: [clang][CodeGen] Implicit Conversion Sanitizer: discover the world of CompoundAssign operators.

I can test this and write a few test cases.

Wed, Oct 31, 2:02 PM · Restricted Project, Restricted Project
lebedev.ri created D53949: [clang][CodeGen] Implicit Conversion Sanitizer: discover the world of CompoundAssign operators.
Wed, Oct 31, 12:02 PM · Restricted Project, Restricted Project
lebedev.ri added a comment to D51570: [X86][XOP] VFRCZ* instructions should be in their own sched class.

Abandon this?

Wed, Oct 31, 10:08 AM
lebedev.ri added a comment to D53935: Delete dependency on config.h.

How does this play with LLVM_ENABLE_THREADS cmake option?

Wed, Oct 31, 8:31 AM
lebedev.ri added a comment to D52695: [clang][Parse] Diagnose useless null statements (PR39111).

Ping.

Wed, Oct 31, 3:53 AM

Tue, Oct 30

lebedev.ri resigned from D35035: [InstCombine] Prevent memcpy generation for small data size.

Not sure what is the general consensus wrt this patch, but i guess it now consistently uses bytes.

Tue, Oct 30, 3:23 PM
lebedev.ri added a comment to D53877: [IR] Strawman for dedicated FNeg IR instruction.

with the intention of adding more Unary operations in the future. E.g. integer Neg (and maybe Abs, Copysign, etc.).

While there are very clear reasons for FNeg to exist, this isn't so true for any of the others listed here..

I don't disagree with that. On the other hand, these do seem like common enough operators to justify retiring the intrinsics and creating IR operators for them.

From what i know, the bar isn't "common enough operator", the bar is more like "too easy to break the existing multi-op pattern".
Funnel shift (rotates) has recently met it, and was added as an operator.
But anyway.

Tue, Oct 30, 12:34 PM
lebedev.ri added a comment to D53869: [NFC][compiler-rt] Cleanup Implicit Conversion Sanitizer tests to use sized types.

Feel like actually marking these two reviews as accepted? :)

Sure! LGTMing now.

Thank you for the review!

Tue, Oct 30, 12:16 PM · Restricted Project
lebedev.ri added a comment to D50251: [compiler-rt][ubsan] Implicit Conversion Sanitizer - integer sign change - compiler-rt part.

LGTMing.

Tue, Oct 30, 12:15 PM · Restricted Project