Fixing build warnings.

[anadon]

I did the mechanical things to fix build warnings, and the changes significantly reduce the warnings on linux. I don't know how that changes will play with windows, but C++ is new-line tolerant so there shouldn't be any issues anyways. I also fixed places where tabs and spaces got mixed up, defaulting to tabs, and converting the equivalent number of spaces into tabs for many files.

As a note, it looks like many of the kernels in src/library/blas/trtri/ have uncanny similarity, so those files should be able to be shrunk down. I would also implore you to purge STRINGIFY.

[pavanky]

You can't unilaterally decide what the style to use consistently all over the place. If you feel like the indentation is inconsistent raise an issue and the maintainers will figure out the best way to fix it. Pushing a 4000 line change is not the right way to handle this situation.

Just send in the changes that will fix the warnings and it will be reviewed.

[anadon]

Many of the things you're considering style throw warnings on linux. There's swaths of code that just needs to be cleaned up to deal with these. While you're worrying about a few thousand removals of '\n' there are still 2907 lines of build output that need to be cut down before I can get to finding out what in my machine or the code base is causing test building to fail. I'm about ready to abandon anything this project has to offer due to how sloppy the code it. It takes those changes to just reduce the error lines put out by 340.

This commit is a direct improvement in code size, readability, and compiler warning/error output. What more do you want out of pull requests?

[pavanky]

There are a lot of warnings, none of them have to do with indentation. It makes it harder to reivew code when you push changes that have nothing to do with each other in to a single monolithic commit.

I already mentioned that if you send the merge that fixes the warnings only, it will be reviewed.

[pavanky]

BTW some of the changes you have made and are suggesting make it seem like you have never written any OpenCL projects of significance. The "\n" exists for a reason and exist in almost all OpenCL libraries.

[anadon]

Where is the documentation for the need of the '\n''s? I am new to openCL, but very familiar with C and those should never be needed in anything claiming C compliance.

[kknox]

Here's an online example/tutorial: https://www.olcf.ornl.gov/tutorials/opencl-vector-addition/

The OpenCL compiler is compiling at 'run-time' (like a JIT), not traditional build-time as conventional host-based compilers do. The opencl compiler needs to be able to see new-lines in the strings because that is how a 'file' is passed to it as input. Removing the \n's would break on various opencl implementations, but you would not know you 'broke' anything until run-time when the kernel is compiled.

[anadon]

"The kernel is the heart of our OpenCL code. The entire kernel must eventually be read in as a c string, the easiest way for a small program like this is to wrap quotes and line returns around the entire kernel. In a real program you would more than likely read the kernel in from a separate file."

If the implementations attempt to claim C compliance and need '\n''s, then they're not C compliant. If that's the case, then the vendors need to fix their interpreters to lex the input correctly.

[pavanky]

@anadon Read more about OpenCL before bugging us about nonsensical suggestions.

[anadon]

You think it's OK to work around broken code by compromising the integrity of your own code? That is one way to tank a project. If you persist pursuing such a misguided approach, anyone wanting to use the code will ultimately be forced to cannibalize it.

If you want an example of something more standard, I've put many hours into my libsuffixarray. This summer if I have to end up using matrix operations, unless this library shapes up, I'll have to reprogram it (the project is too broken to rely on and it is a total failure) or cannibalize it (a few algorithms were good, but mostly junk).

If you ego gets between you and making code of sufficient quality, get out of open source. Here, expect to be called out on everything. Everything to be nitpicked to death. And by the end of it, the project dies, or is of such quality that to never really gets reimplemented again.

And if think I'm harsh, try dealing with the LKML, or the Gnu people. They've gotten good enough to set the standards because most programmers can't compete with them. Sure new approaches like LLVM pop up, but LLVM's codebase is wild, unmanaged, and makes much of it's development a hellscape that is weighing down it's development.

[pavanky]

If you don't like the source go ahead and do your own thing. Or talk to people constructively.

I am not from amd but and at times I have been annoyed with the complexity myself. That doesn't mean I go about changing code without consulting the team.

I have been communicating with the clmathlibraries team for close to 3 years now and they have been nothing but professional and open to criticism /changes.

You come in here with your "fixes" that shouldn't be committed together and complain like a child when someone suggests that is not how projects are handled and there are reasons why things are the way they are.

You are the kind of person that makes organizations think twice about open sourcing.

@kknox I'm sorry if I dragged the issue further than necessary. I'm unsubscribing from the issue so I am not involved in this discussion further.

[kknox]

@pavanky Thank you for your input and your time; you don't have stay involved in the thread.

@anadon

This commit is a direct improvement in code size, readability, and compiler warning/error output. What more do you want out of pull requests?

Separate commits that change 'style' with commits that change 'substance'. We review any incoming PR. For PR's that change 'substance', we don't want our diff/merge tools to highlight all the 'style' changes. For PR's that change style, document the goal that you are trying to achieve, and i'm sure the maintainers will consider your change. There is no guarantee that a maintainer will merge your work, but it helps to state why you want to make the changes. If you are fixing a file to have consistent spacing/tabbing, that will likely be better received that if you are arbitrarily changing variable names. Regardless, document your reasoning, and the maintainers can look at the diffs and attempt to make sure there are no unintended side effects from the changes. We are very lucky @pavanky saw you tried to eliminate the \n's from the kernels.

If the implementations attempt to claim C compliance and need '\n''s, then they're not C compliant. If that's the case, then the vendors need to fix their interpreters to lex the input correctly. .... You think it's OK to work around broken code by compromising the integrity of your own code?

It don't think you understand yet; we are not working around broken code. Traditionally, when code is written in a .cpp or implementation file of some kind, newlines are embedded in the code. If you open the file in binary, you will parse the \n (hex 0x0a) character even though you don't see the character in an editor. However, our libraries are not compiling opencl code from a file, they generate opencl programs on the fly from strings. Those strings need to emulate carriage returns 'manually', because by C standards they are only null terminated. You could make the argument, that C programs should compile fine written out as a single line in a .cpp file, but I think you will find that you run into subtle problems. In addition, single line programs render a debugger entirely useless. Thats good for code obfuscation, but not what we desire for our libraries.

If you ego gets between you and making code of sufficient quality, get out of open source. Here, expect to be called out on everything. Everything to be nitpicked to death.

I don't think anybody on this (extended) team has shown ego. It's fine to nitpick and criticize, and if made with good intentions we welcome and consider it. The code originally developed was never intended to be open sourced. We originally released the project as binary only files as clAmdBlas. It was the hard work, by several forward looking people inside of AMD that pushed passed all the legal and political hurdles in our company to get our math libraries open. It has benefited the libraries in numerous ways. This was the only viable way to get the math libraries to compile and run on OSX. The integration with the cloud based build and test farms is amazing. Opening the clmath libraries also set an early precedent within my company.

And if think I'm harsh, try dealing with the LKML, or the Gnu people. They've gotten good enough to set the standards because most programmers can't compete with them.

They are not a model I desire to emulate.

[anadon]

I'll take your 19 roll on diplomacy. I'll work on sorting out formatting from content changes. Also, I think double quotes around a multi-line string preserves the newlines, but I'll look into that. I've also remove some unnecessary branching in a few of the kernels that I caught.

[anadon]

And C++0x comes to the rescue again: http://stackoverflow.com/questions/1135841/c-multiline-string-literal