hfinkel (Hal Finkel)
User

Projects

User does not belong to any projects.

User Details

User Since
Nov 16 2012, 6:02 PM (248 w, 3 d)

Recent Activity

Today

hfinkel committed rL311458: [test-suite] Fix warnings in Pathfinder.
[test-suite] Fix warnings in Pathfinder
Tue, Aug 22, 7:49 AM

Yesterday

hfinkel added a comment to D36772: Unmerge GEPs to reduce register pressure on IndirectBr edges..

With this patch, the Python interpreter loop runs faster by ~5%.

Mon, Aug 21, 6:45 PM
hfinkel committed rL311413: [test-suite] Adding the Pathfinder Benchmark.
[test-suite] Adding the Pathfinder Benchmark
Mon, Aug 21, 5:40 PM
hfinkel closed D36680: [test-suite] Adding Pathfinder Benchmark by committing rL311413: [test-suite] Adding the Pathfinder Benchmark.
Mon, Aug 21, 5:39 PM
hfinkel added a comment to D36680: [test-suite] Adding Pathfinder Benchmark.

A couple of things I'll fix while committing...

Mon, Aug 21, 5:31 PM
hfinkel committed rL311411: [test-suite] Adding the miniFE Benchmark.
[test-suite] Adding the miniFE Benchmark
Mon, Aug 21, 4:55 PM
hfinkel closed D36683: [test-suite] Adding miniFE Benchmark by committing rL311411: [test-suite] Adding the miniFE Benchmark.
Mon, Aug 21, 4:55 PM
hfinkel committed rL311398: [test-suite] Adding the miniAMR Benchmark.
[test-suite] Adding the miniAMR Benchmark
Mon, Aug 21, 3:53 PM
hfinkel closed D36682: [test-suite] Adding miniAMR Benchmark by committing rL311398: [test-suite] Adding the miniAMR Benchmark.
Mon, Aug 21, 3:53 PM
hfinkel added inline comments to D17080: [LAA] Allow more run-time alias checks by coercing pointer expressions to AddRecExprs.
Mon, Aug 21, 1:42 PM
hfinkel added a comment to D36928: [Polly][MatMul][WIP] Disable the Loop Vectorizer.

I suggest that you wait on this until we understand the regression.

In part, it's possible that this is the wrong fix. Based on the bug report, it is possible that the problem is that the vectorizer is generating runtime checks. Maybe aliasing metadata would help. Maybe it's unrolling too much.

Currently, in case of GEMM detected and optimized by Polly, only the SLP vectorizer is needed out of two LLVM vectorizers. Usually the Loop Vectorizer doesn't affect the code. Consequently, as far as I understand, this fix shouldn't hurt anything.

To generate BLIS micro-kernel [1], we apply tiling and unroll the two innermost loops. Subsequently, we use LICM to sink and hoist all stores and loads form the innermost loop, and apply the SLP vectorizer to get a sequence of rank-1 updates.

I think that the application of the Loop Vectorizer instead of the SLP Vectorizer is the way to improve matrix optimization of Polly. However, as far as I understand, there is no way to make the Loop Vectorizer vectorize a loop and sink and hoist accesses. All available metadata are only optimization hints and the optimizer will only interleave and vectorize loops if it believes it's safe to do so.

Mon, Aug 21, 1:11 PM
hfinkel added a comment to D36928: [Polly][MatMul][WIP] Disable the Loop Vectorizer.

Hi Hal,

I think this is conceptually the right approach. We currently generate code -- with explicit register unrolling -- and expect the SLP vectorizer to perform the vectorization. I believe communicating this information via explicit metadata is reasonable.

We may want to move towards using the LLVM loop vectorizer rather than the SLP vectorizer, but this requires both changes to the loop vectorizer and to our code generation strategy. We should certainly consider this, but I feel that this could be separate steps. 1) clarify current behavior and fix regressions, 2) expand the loop vectorizer, 3) change our code generation logic.

Mon, Aug 21, 1:09 PM
hfinkel added inline comments to D36768: [test-suite] Add -i option to fpcmp to ignore whitespace changes..
Mon, Aug 21, 12:18 PM
hfinkel added a comment to D36896: [TargetTransformInfo] Call target getMemoryOpCost for LoadInst from getUserCost.

We currently have two cost models in TTI. One is for the vectorizer and it is supposed to measure normalized reciprocal throughputs. The other model, the "user cost" model, is used by the inliner, the loop unroller, and a few other things. This model measures some less-well-defined combination of performance factors (thoughput/latency) and code size. I don't know whether tying these things together makes sense. At the risk of going down some rabbit hole, we should probably answer that question. If it does make sense, we should tie them together more completely than just for memory operations.

Mon, Aug 21, 9:48 AM
hfinkel added inline comments to D36768: [test-suite] Add -i option to fpcmp to ignore whitespace changes..
Mon, Aug 21, 9:31 AM

Sat, Aug 19

hfinkel added a comment to D36928: [Polly][MatMul][WIP] Disable the Loop Vectorizer.

I suggest that you wait on this until we understand the regression.

Sat, Aug 19, 11:52 AM
hfinkel added a comment to D36928: [Polly][MatMul][WIP] Disable the Loop Vectorizer.

I suggest that you wait on this until we understand the regression.

Sat, Aug 19, 11:50 AM

Fri, Aug 18

hfinkel added a comment to D36917: [XRay] [test-suite] Upgrade Google Benchmark library to 1.2.0.

For context, as I noted in PR34215, the version of the Google benchmark library in the test suite, MicroBenchmarks/libs/benchmark-1.1.0, doesn't build with Clang trunk (https://github.com/google/benchmark/issues/409). Upgrading to 1.2.0 should fix this.

Fri, Aug 18, 9:10 PM
hfinkel accepted D36736: [PowerPC] Check if the pre-increment preparations have already been made so that they are not done twice.

LGTM

Fri, Aug 18, 11:49 AM

Thu, Aug 17

hfinkel added inline comments to D36736: [PowerPC] Check if the pre-increment preparations have already been made so that they are not done twice.
Thu, Aug 17, 9:26 AM

Wed, Aug 16

hfinkel committed rL311043: Don't use -no-integrated-as in test/Driver/opt-record.c.
Don't use -no-integrated-as in test/Driver/opt-record.c
Wed, Aug 16, 2:52 PM
hfinkel committed rL311041: Base optimization-record file names on the final output.
Base optimization-record file names on the final output
Wed, Aug 16, 2:35 PM
hfinkel updated the diff for D32198: [TySan] A Type Sanitizer (LLVM).

Rebased.

Wed, Aug 16, 9:53 AM
hfinkel committed rL311014: [BDCE] Don't check demanded bits on unsized types.
[BDCE] Don't check demanded bits on unsized types
Wed, Aug 16, 9:10 AM
hfinkel added a comment to D36629: [test-suite]: Adding lcals proxy-app.

I think that there are two or three ways to do this reasonably:

  1. Split this into multiple parts (e.g., similar to how I did TSVC).
  2. Make this use the Google benchmarking library (we have a copy in MicroBenchmarks/libs/benchmark-1.1.0 already), although we'd need some enhancement to litsupport (or an LNT plugin) in order to get separate timing outputs for each kernel.
  3. Like (2), but use the builtin timing infrastructure and parse that output.
Wed, Aug 16, 7:57 AM
hfinkel added a comment to D36628: [test-suite]: Adding Lulesh Proxy-app.

This also needs support for the old Makefile-based build system. See https://reviews.llvm.org/rL310951, for example, and also http://llvm.org/svn/llvm-project/test-suite/trunk/MultiSource/Benchmarks/DOE-ProxyApps-C/XSBench/Makefile and http://llvm.org/svn/llvm-project/test-suite/trunk/MultiSource/Benchmarks/DOE-ProxyApps-C++/PENNANT/Makefile

Wed, Aug 16, 7:56 AM
hfinkel added a comment to D36626: [test-suite]: Adding RSBench proxy-app..

This also needs support for the old Makefile-based build system. See https://reviews.llvm.org/rL310951, for example, and also http://llvm.org/svn/llvm-project/test-suite/trunk/MultiSource/Benchmarks/DOE-ProxyApps-C/XSBench/Makefile and http://llvm.org/svn/llvm-project/test-suite/trunk/MultiSource/Benchmarks/DOE-ProxyApps-C++/PENNANT/Makefile

Wed, Aug 16, 7:56 AM
hfinkel added a comment to D36621: [test-suite]: Adding SimpleMOC proxy-app..

This also needs support for the old Makefile-based build system. See https://reviews.llvm.org/rL310951, for example, and also http://llvm.org/svn/llvm-project/test-suite/trunk/MultiSource/Benchmarks/DOE-ProxyApps-C/XSBench/Makefile and http://llvm.org/svn/llvm-project/test-suite/trunk/MultiSource/Benchmarks/DOE-ProxyApps-C++/PENNANT/Makefile

Wed, Aug 16, 7:56 AM

Tue, Aug 15

hfinkel committed rL310993: [test-suite] Adding a Makefile for PENNANT.
[test-suite] Adding a Makefile for PENNANT
Tue, Aug 15, 11:25 PM
hfinkel added a comment to D36629: [test-suite]: Adding lcals proxy-app.

I think that there are two or three ways to do this reasonably:

Tue, Aug 15, 9:12 PM
hfinkel accepted D35188: Add bitreverse LNT benchmark..

LGTM

Tue, Aug 15, 8:55 PM
hfinkel added inline comments to D36736: [PowerPC] Check if the pre-increment preparations have already been made so that they are not done twice.
Tue, Aug 15, 8:51 PM
hfinkel added a comment to D36699: [test-suite] Adding miniGMG.

Changed timer.x86.c as commented and also adjusted for the rand/srand function usage

Tue, Aug 15, 5:31 PM
hfinkel committed rL310975: [test-suite] Adding PENNANT.
[test-suite] Adding PENNANT
Tue, Aug 15, 4:46 PM
hfinkel closed D36709: [test-suite] Adding PENNANT by committing rL310975: [test-suite] Adding PENNANT.
Tue, Aug 15, 4:46 PM
hfinkel added inline comments to D36628: [test-suite]: Adding Lulesh Proxy-app.
Tue, Aug 15, 3:57 PM
hfinkel added a comment to D36682: [test-suite] Adding miniAMR Benchmark.

Substituted in the glib_compat_rand implementation to remove dependency on libc's rand.

Tue, Aug 15, 3:56 PM
hfinkel added a comment to D36718: [test-suite] Adding CLAMR.

Oops, wrong diff file uploaded before.

Adjusted for rand/srand/drand48/srand48 usage.

Tue, Aug 15, 3:56 PM
hfinkel added a comment to D36626: [test-suite]: Adding RSBench proxy-app..

This test does not have a numerical verification output, but nevertheless, I'd prefer that we not depend explicitly on libc's rand. Different libc implementations have different implementations of rand, and I'd rather not have that be a cause of any latter difficultly tracking down performance differences between systems/configurations.

The easiest way to deal with this is probably to substitute our own rand implementation. I wrote one of these for the original XSBench patch (see glibc_compat_rand.{c.h} in https://reviews.llvm.org/D27311). Can you grab those files from there

Tue, Aug 15, 3:55 PM
hfinkel added a comment to D36621: [test-suite]: Adding SimpleMOC proxy-app..

This test does not have a numerical verification output, but nevertheless, I'd prefer that we not depend explicitly on libc's rand. Different libc implementations have different implementations of rand, and I'd rather not have that be a cause of any latter difficultly tracking down performance differences between systems/configurations.

The easiest way to deal with this is probably to substitute our own rand implementation. I wrote one of these for the original XSBench patch (see glibc_compat_rand.{c.h} in https://reviews.llvm.org/D27311). Can you grab those files from there

Tue, Aug 15, 3:54 PM
hfinkel added a reviewer for D36683: [test-suite] Adding miniFE Benchmark: dberlin.
Tue, Aug 15, 3:01 PM
hfinkel added inline comments to D36683: [test-suite] Adding miniFE Benchmark.
Tue, Aug 15, 3:01 PM
hfinkel committed rL310951: [test-suite] Adding the HPCCG benchmark.
[test-suite] Adding the HPCCG benchmark
Tue, Aug 15, 12:54 PM
hfinkel closed D36582: [test-suite] Adding HPCCG benchmark by committing rL310951: [test-suite] Adding the HPCCG benchmark.
Tue, Aug 15, 12:54 PM
hfinkel added inline comments to D36718: [test-suite] Adding CLAMR.
Tue, Aug 15, 11:43 AM
hfinkel added inline comments to D36699: [test-suite] Adding miniGMG.
Tue, Aug 15, 11:30 AM
hfinkel added inline comments to D36682: [test-suite] Adding miniAMR Benchmark.
Tue, Aug 15, 11:07 AM
hfinkel added inline comments to D36628: [test-suite]: Adding Lulesh Proxy-app.
Tue, Aug 15, 10:36 AM
hfinkel added a comment to D36621: [test-suite]: Adding SimpleMOC proxy-app..

This test does not have a numerical verification output, but nevertheless, I'd prefer that we not depend explicitly on libc's rand. Different libc implementations have different implementations of rand, and I'd rather not have that be a cause of any latter difficultly tracking down performance differences between systems/configurations.

Tue, Aug 15, 10:31 AM
hfinkel added a comment to D36626: [test-suite]: Adding RSBench proxy-app..

This test does not have a numerical verification output, but nevertheless, I'd prefer that we not depend explicitly on libc's rand. Different libc implementations have different implementations of rand, and I'd rather not have that be a cause of any latter difficultly tracking down performance differences between systems/configurations.

Tue, Aug 15, 10:30 AM
hfinkel added reviewers for D36621: [test-suite]: Adding SimpleMOC proxy-app.: MatzeB, kristof.beyls.
Tue, Aug 15, 9:57 AM
hfinkel added reviewers for D36626: [test-suite]: Adding RSBench proxy-app.: MatzeB, kristof.beyls.
Tue, Aug 15, 9:56 AM
hfinkel added reviewers for D36628: [test-suite]: Adding Lulesh Proxy-app: kristof.beyls, MatzeB.
Tue, Aug 15, 9:56 AM
hfinkel added reviewers for D36629: [test-suite]: Adding lcals proxy-app: MatzeB, kristof.beyls.
Tue, Aug 15, 9:55 AM
hfinkel added reviewers for D36680: [test-suite] Adding Pathfinder Benchmark: MatzeB, kristof.beyls.
Tue, Aug 15, 9:55 AM
hfinkel added reviewers for D36682: [test-suite] Adding miniAMR Benchmark: MatzeB, kristof.beyls.
Tue, Aug 15, 9:54 AM
hfinkel added reviewers for D36683: [test-suite] Adding miniFE Benchmark: MatzeB, kristof.beyls.
Tue, Aug 15, 9:54 AM
hfinkel added reviewers for D36699: [test-suite] Adding miniGMG: kristof.beyls, MatzeB.
Tue, Aug 15, 9:53 AM
hfinkel added reviewers for D36709: [test-suite] Adding PENNANT: kristof.beyls, MatzeB.
Tue, Aug 15, 9:53 AM
hfinkel added reviewers for D36718: [test-suite] Adding CLAMR: kristof.beyls, MatzeB.
Tue, Aug 15, 9:52 AM
hfinkel added reviewers for D36738: [test-suite] Adding miniXyce Benchmark: kristof.beyls, MatzeB.
Tue, Aug 15, 9:51 AM

Mon, Aug 14

hfinkel updated the diff for D22189: llvm.noalias - Clang CodeGen - check restrict variable map only for restrict-qualified lvalues.

Rebased.

Mon, Aug 14, 9:28 PM
hfinkel updated the diff for D9403: llvm.noalias - Clang CodeGen for local restrict-qualified pointers.

Rebased and addressing review comments.

Mon, Aug 14, 9:27 PM
hfinkel added inline comments to D9403: llvm.noalias - Clang CodeGen for local restrict-qualified pointers.
Mon, Aug 14, 9:26 PM
hfinkel updated the diff for D9401: llvm.noalias - The AA implementaton.

Rebased.

Mon, Aug 14, 8:00 PM
hfinkel updated the diff for D9375: An llvm.noalias intrinsic.

Rebased.

Mon, Aug 14, 7:59 PM
hfinkel updated the diff for D9386: llvm.noalias - CaptureTracking needs to look through them.

Rebased.

Mon, Aug 14, 7:59 PM
hfinkel updated the diff for D9400: llvm.noalias - Use noalias intrinsics when inlining (and update them when cloning metadata).

Rebased.

Mon, Aug 14, 7:56 PM
hfinkel updated the diff for D9378: llvm.noalias - Add IRBuilder support.

Rebased.

Mon, Aug 14, 7:55 PM
hfinkel updated the diff for D22184: llvm.noalias - don't look through it to simplify GEPs.

Rebased.

Mon, Aug 14, 7:55 PM
hfinkel updated the diff for D9382: llvm.noalias - don't prevent loop vectorization.

Rebased.

Mon, Aug 14, 7:55 PM
hfinkel updated the diff for D9380: llvm.noalias - CodeGen support.

Rebased.

Mon, Aug 14, 7:52 PM
hfinkel updated the diff for D9376: llvm.noalias - handling of dead intrinsics.

Rebased.

Mon, Aug 14, 7:52 PM
hfinkel updated the diff for D9379: llvm.noalias - don't interfere with llvm.assume.

Rebased.

Mon, Aug 14, 7:51 PM
hfinkel updated the diff for D9377: llvm.noalias - don't block EarlyCSE.

Rebased.

Mon, Aug 14, 7:50 PM
hfinkel updated the diff for D9398: llvm.noalias - GetUnderlyingObjects to optionally collect noalias calls.

Rebased.

Mon, Aug 14, 7:49 PM
hfinkel updated the diff for D32197: [TySan] A Type Sanitizer (Runtime Library).

Rebased.

Mon, Aug 14, 7:03 PM
hfinkel updated the diff for D32198: [TySan] A Type Sanitizer (LLVM).

Rebased.

Mon, Aug 14, 7:03 PM
hfinkel added inline comments to D32197: [TySan] A Type Sanitizer (Runtime Library).
Mon, Aug 14, 6:59 PM
hfinkel updated the diff for D32199: [TySan] A Type Sanitizer (Clang).

Rebased.

Mon, Aug 14, 6:56 PM
hfinkel added a comment to D9401: llvm.noalias - The AA implementaton.

Hi Hal, I found an issue with this part when analyzing two pointers that are 'derived from each other', but are used in different loops.
Following code would go wrong:

int* restrict p=malloc(sizeof(int)*20);
int i;
for (i=0; i<20; ++i) {p[i]=i; }
for (i=0; i<20; ++i) { if (p[i] != i) should_never_happen(); }

Do you think that the proposed fix is ok ?

Mon, Aug 14, 6:23 PM
hfinkel added inline comments to D36113: [Loop Vectorize] Vectorize Loops with Backward Dependence.
Mon, Aug 14, 12:11 PM
hfinkel added inline comments to D35188: Add bitreverse LNT benchmark..
Mon, Aug 14, 10:34 AM
hfinkel committed rL310859: [ValueTracking] Don't delete assumes of side-effectful instructions.
[ValueTracking] Don't delete assumes of side-effectful instructions
Mon, Aug 14, 10:13 AM
hfinkel closed D36590: [ValueTracking] don't delete assumes of side-effectful instructions by committing rL310859: [ValueTracking] Don't delete assumes of side-effectful instructions.
Mon, Aug 14, 10:12 AM

Sun, Aug 13

hfinkel accepted D34245: [PowerPC] Refine the checking for emiting TOC restore nops and Tail-Call eligibility..

LGTM

Sun, Aug 13, 12:26 AM

Sat, Aug 12

hfinkel accepted D36592: [BDCE] clear poison generators after turning a value into zero (PR33695, PR34037).

LGTM

Sat, Aug 12, 8:00 AM
hfinkel added a comment to D36592: [BDCE] clear poison generators after turning a value into zero (PR33695, PR34037).

The comments about assume, otherwise, LGTM too.

Sat, Aug 12, 7:37 AM

Fri, Aug 11

hfinkel added a comment to D35851: [Dominators] Include infinite loops in PostDominatorTree.

I think that we should go ahead with this approach.

Fri, Aug 11, 7:22 PM
hfinkel accepted D36220: [Loop Vectorize] Added a separate metadata .

LGTM

Fri, Aug 11, 1:25 PM
hfinkel added a comment to D36582: [test-suite] Adding HPCCG benchmark.

Thanks for the feedback. I have updated the LICENSE.TXT file to include this application. The machine I used was an IBM x3550 M4.

Fri, Aug 11, 11:39 AM
hfinkel added inline comments to D36621: [test-suite]: Adding SimpleMOC proxy-app..
Fri, Aug 11, 10:04 AM
hfinkel added a comment to D36582: [test-suite] Adding HPCCG benchmark.

Hi @homerdin,

Great to see you taking the effort to make the test-suite more relevant!
I think it'd be helpful if you could also add a little explanation of how adding this code makes the test-suite more relevant.
I'm assuming that this kind of code is common in some domain (HPC?) and that there is no other benchmark already in the test-suite with similar enough properties?

Fri, Aug 11, 10:02 AM
hfinkel added inline comments to D36220: [Loop Vectorize] Added a separate metadata .
Fri, Aug 11, 9:11 AM
hfinkel added inline comments to D36592: [BDCE] clear poison generators after turning a value into zero (PR33695, PR34037).
Fri, Aug 11, 9:00 AM

Thu, Aug 10

hfinkel added a comment to D35851: [Dominators] Include infinite loops in PostDominatorTree.

@hfinkel : Hi Hal, we discussed this topic for a while in Paris. I would very much appreciate your opinion on this patch.

Thu, Aug 10, 10:46 PM
hfinkel accepted D36590: [ValueTracking] don't delete assumes of side-effectful instructions.

LGTM

Thu, Aug 10, 8:09 PM
hfinkel added inline comments to D36592: [BDCE] clear poison generators after turning a value into zero (PR33695, PR34037).
Thu, Aug 10, 7:50 PM
hfinkel added a comment to D29660: [OpenMP] Add flag for overwriting default PTX version for OpenMP targets.

First of all, I apologize if I've upset you with my previous post. I am actively working on understanding what is causing these issues. It is not my intention to write tests that work on local configurations only. I am upset to see that these tests keep failing for your and maybe other configurations. Without knowing the actual reason of the failures I can only speculate what is going wrong with them hence the flurry of changes.

Thu, Aug 10, 8:48 AM

Wed, Aug 9

hfinkel added inline comments to D36113: [Loop Vectorize] Vectorize Loops with Backward Dependence.
Wed, Aug 9, 10:07 PM